This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 1/2] Emit inferior, thread and frame selection events to all UIs
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>, Simon Marchi <simon dot marchi at polymtl dot ca>, <gdb-patches at sourceware dot org>
- Cc: Antoine Tremblay <antoine dot tremblay at ericsson dot com>
- Date: Mon, 3 Oct 2016 13:39:48 -0400
- Subject: Re: [PATCH v3 1/2] Emit inferior, thread and frame selection events to all UIs
- Authentication-results: sourceware.org; auth=none
- Authentication-results: spf=none (sender IP is ) smtp.mailfrom=simon dot marchi at ericsson dot com;
- References: <20160924201331.23605-1-simon.marchi@polymtl.ca> <906fc6d1-01f2-c81b-2ff9-ef11b787ec9e@redhat.com>
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
On 16-10-03 12:47 PM, Pedro Alves wrote:
> Hi Simon,
>
> This looks good to me now, module a couple minor issues pointed out
> below. Fix these and you're good to go.
>
> On 09/24/2016 09:13 PM, Simon Marchi wrote:
>> @@ -1885,7 +1901,19 @@ void
>> cmd_func (struct cmd_list_element *cmd, char *args, int from_tty)
>> {
>> if (cmd_func_p (cmd))
>> - (*cmd->func) (cmd, args, from_tty);
>> + {
>> + struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
>> +
>> + if (cmd->suppress_notification != NULL)
>> + {
>> + cleanups = make_cleanup_restore_integer (cmd->suppress_notification);
>
> This will incorrectly leave the null_cleanup not run.
> You should not overwrite "cleanups". Should be just:
>
> + make_cleanup_restore_integer (cmd->suppress_notification);
Done, thanks.
>> + *cmd->suppress_notification = 1;
>> + }
>> +
>> + (*cmd->func) (cmd, args, from_tty);
>> +
>> + do_cleanups (cleanups);
>
>
>
>> + else /* MI_COMMAND */
>> + {
>> + if (strcmp (command->command, "interpreter-exec") == 0
>> + && command->argc > 1)
>> +
>
> This empty line here made me pause and think that the code
> looks suspicious. Better would be to wrap the then/else blocks
> in {}s, since they're multi-line.
Right, it's clearer.
>> +@item =thread-selected,id="@var{id}"[,frame="@var{frame}"]
>> +Informs that the selected thread or frame were changed. This notification
>> +is not emitted as result of the @code{-thread-select} or
>> +@code{-stack-select-frame} commands, but is emitted whenever an MI command
>> +that is not documented to change the selected thread and frame actually
>> +changes them. In particular, invoking, directly or indirectly
>> +(via user-defined command), the CLI @code{thread} or @code{frame} commands,
>> +will generate this notification. Changing the thread of frame from another
>> +user interface (see @ref{Interpreters}) will also generate this notification.
>> +
>
> Typo: s/thread of frame/thread or frame/
Woops, thanks.
I'll give it a last complete test run before pushing.
Simon