Re: [RFA] Implement MI notification for new threads.

[Vladimir Prus <vladimir at codesourcery dot com>]

On Friday 14 March 2008 18:32:06 Daniel Jacobowitz wrote:
> On Sat, Mar 08, 2008 at 09:26:31PM +0300, Vladimir Prus wrote:
> > 
> > This patch implements reporting about new threads using MI
> > 'notify-async-output' channel, like this:
> > 
> > 
> > 	~"143\t  if (pthread_create...
> > 	^done
> > 	(gdb)
> > 	n
> > 	&"n\n"
> > 	~"[New Thread 0xb7e36b90 (LWP 23024)]\n"
> > 	=thread-created,id=2
> > 	~"148\t  if (verbose) printf (\"Made thread %d\\n\", tid1);\n"
> > 	^done
> 
> Sounds reasonable to me.  In MI mode (current interpreter MI, not
> necessarily top level MI) should we be printing the ~"[New Thread]"
> message?

Well, we better print that message via observer, installed only
when CLI is the top-level interpreter. But it does not hurt, on
the other hand.

> > First, we want those notification to be always printed, if GDB is started
> > in MI mode. If we fail to do that, then MI frontend might miss a notification
> > caused by CLI command. Therefore, it is best to register notification hooks
> > only when creating the top-level interpreter, and don't touch the hook if
> > we temporary switch interpreter. In order to make it possible, I've added
> > a 'top_level' flag to interpreter's initialization function.
> > 
> > Also, I'm using observers to report new thread event. Unfortunately, observers
> > does not allow to specify 'closure' that will be passed back to the callback, and
> > therefore, callback for the new thread observer does not have an easy way to get
> > MI interpreter data. I've introduced new function to get top-level's interpreter
> > data to fix this. 
> 
> These both sound reasonable to me.  Adding a closure to observers
> would be reasonable also.
> 
> > +@deftypefun void new_thread (struct thread_info *@var{t})
> > +The thread specified by @var{objfile} has been created.
> > +@end deftypefun
> 
> Copy-paste leftover, @var{t}.
> 
> > +   MI is the top-level interpreter, then it will always report
> > +   events such a target stops and new thread creation, even if they
> > +   are caused by CLI commands.  */
> 
> "such as"
> 
> > @@ -462,6 +476,13 @@ interpreter_completer (char *text, char *word)
> >    return matches;
> >  }
> >  
> > +extern void *top_level_interpreter_data ()
> 
> Newline, (void).
> 
> > +{
> > +  if (top_level_interpreter)
> > +    return top_level_interpreter->data;
> > +  return NULL;
> 
> gdb_assert (top_level_interpreter) ?

Yes. We should always have top level interpreter anyway.

> 
> > +/* Returns opaque data associated with the top-level interpreter.  */
> > +extern void *top_level_interpreter_data ();
> 
> void.
> 
> I believe this actually matters in C - a function without the (void)
> is technically unprototyped and the compiler will let you pass
> arguments to it.

Yes, I know.

> Otherwise OK to commit.  

Thanks, I've revised per comments above and checked in.

> Needs some documentation, please, and a 
> test case would be a good idea.

I shall work on that now.

> Jim did some work on notification events for the remote protocol;
> maybe we can dust that off and get this to work in gdbserver too.

Probably.

- Volodya