Re: [patch:MI] Observer for thread-changed

[Pedro Alves <pedro at codesourcery dot com>]

A Monday 09 June 2008 23:50:05, Nick Roberts wrote:
> Pedro Alves writes:
>  > Sorry, if I missed the discussion on it, but,
>  >
>  > A Monday 09 June 2008 13:16:09, Nick Roberts wrote:
>  > > ? ?annotate_thread_changed ();
>  > > ? ?gdb_thread_select (uiout, tidstr, NULL);
>  > > + ?observer_notify_thread_changed ();
>  > > ?}
>  >
>  > This is conceptually not right. gdb_thread_select is a libgdb
>  > function, that filters exceptions.  If do_captured_thread_select
>  > throws an error, you will still call the observer.  Plus,
>  > do_captured_thread_select is already printing the thread change
>  > to MI, which means you'll get the output twice now, in MI?
>
> I don't think that's a problem.  Removing the output from -thread-select
> would make it backwardly incompatible.

>  > Why not call the observer from inside do_captured_thread_select,
>  > instead of on both CLI and MI commands?
>
> Yes, you're right.  Presumably I could also put it in a clause in
> gdb_thread_select?  I did have it in do_captured_thread_select in the first
> patch but I moved it.  I can't fully explain why now but I think I must
> have got confused by output of the frame_changed observer, which was also
> part of that patch, being triggered by "info threads".
>

Right, and this happens due to the overload GDB makes on
internally selected thread/frame, and user selected thread/frame.
More on that Real Soon Now (TM)...  Stay tuned.

> > Also, it may make sense to add a "reason" parameter to
> > the observer, as in "changed due to user/frontend request", or
> > "due to a stop event", but that's not actually required right now.
>
> I'm not sure what use this information would be.  If it's due to a stop
> event then the reason should be given in the async output.

It all amounts to:

 - should there be an MI async event on -thread-select if the
   reply already carries that information?
 - if a command requires a synchronous reply, then it should be
   implemented in the command itself, not in an observer.

> How about the change below instead?  This, of course, requires no change to
> mi-main.c.

I'd really prefer to keep gdb_thread_select just an exception
wrapper, and do the observer call in do_captured_thread_select.

-- 
Pedro Alves