[RFA] Use observers to report stop events.
Vladimir Prus
vladimir@codesourcery.com
Tue Jun 10 11:58:00 GMT 2008
On Thursday 05 June 2008 19:41:11 Daniel Jacobowitz wrote:
> On Sun, May 04, 2008 at 12:25:54PM +0400, Vladimir Prus wrote:
> > Are those two new patches, together with the previously posted one
> > (changing stop_observer not to always fire) OK?
>
> For this patch, if an error occurs during proceed (i.e. if the
> cleanup restoring suppress_normal_stop_observer is run), will the
> normal_stop observer ever be called? Is that a problem?
You mean this block:
old_cleanups2 = make_cleanup_restore_integer
(&suppress_normal_stop_observer, 1);
proceed (real_pc, TARGET_SIGNAL_0, 0);
do_cleanups (old_cleanups2);
If proceed throws, before calling normal_stop, we'll get back to event loop,
and run cleanup. We won't call the observer. It's an issue if we've printed
"*running" and thrown after after. However, it's the issue we have now, as
well -- we print ^running even before calling proceed, and if something later
throws, we'll never print *stopped. Possible solutions are:
- Require that frontend refresh thread state on ^error
- Emit *stopped if exception is thrown (this requires checking that the
target is actually stopped, if exception is thrown).
> > + If the argument is pointer to allocated memory, then you need to
>
> is a pointer
>
> > struct cleanup *
> > -make_my_cleanup (struct cleanup **pmy_chain, make_cleanup_ftype *function,
> > - void *arg)
> > +make_cleanup_restore_integer (int *variable, int value)
> > +{
> > + struct restore_integer_closure *c =
> > + xmalloc (sizeof (struct restore_integer_closure));
> > + struct cleanup *cleanup = make_cleanup (restore_integer, (void *) c);
> > + c->variable = variable;
> > + c->value = *variable;
> > + *variable = value;
> > + cleanup_chain->free_arg = xfree;
> > + return cleanup;
> > +}
>
> Could you use make_my_cleanup2 here to avoid poking around in
> cleanup_chain, please?
Done.
>
> Also, the only thing value is used for is to set *variable. I
> suggest not setting the variable in a function named
> "make_cleanup_restore_integer", which doesn't say anything about
> setting. So I would prefer this:
>
> old_cleanup = make_cleanup_restore_integer (some_var);
> some_var = 0;
Done.
>
> > * infrun.c (finish_command): Don't pass cleanup
> > to continuation.
> > (finish_command_continuation): Don't grab cleanup from
> > the passed data, as we don't use, and cannot, use it anyway.
>
> OK.
Thanks, checked in. Since there were some adjustment, I attach
the final versions of the patches as checked in.
- Volodya
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Introduce-common-cleanup-for-restoring-integers.patch
Type: text/x-diff
Size: 5656 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20080610/5e24b119/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Suppress-normal-stop-observer-when-it-s-problematic.patch
Type: text/x-diff
Size: 4077 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20080610/5e24b119/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Remove-stale-code.patch
Type: text/x-diff
Size: 1611 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20080610/5e24b119/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Use-observers-to-report-stop-events-in-MI.patch
Type: text/x-diff
Size: 16352 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20080610/5e24b119/attachment-0003.bin>
More information about the Gdb-patches
mailing list