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

[Daniel Jacobowitz <drow at false dot org>]

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?

> 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) ?

> +/* 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.

Otherwise OK to commit.  Needs some documentation, please, and a
test case would be a good idea.

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.

-- 
Daniel Jacobowitz
CodeSourcery