Re: [PATCH 2/2] Add "set/show debug unwinder" prefix commands.

[Samuel Bronson <naesten at gmail dot com>]

At Thu, 04 Jun 2009 16:05:06 -0600,
Tom Tromey wrote:
> 
> >>>>> "Samuel" == Samuel Bronson <naesten@gmail.com> writes:
> 
> Samuel> Also add one subcommand for tracing the sniffing of stack
> Samuel> frames by unwinders.
> 
> This is also basically ok; just needs a ChangeLog and some formatting
> fixlets, nothing serious.
> 
> Samuel> +/* trace unwinders called */
> Samuel> +static int trace_unwinders;
> 
> In the GNU style, comments should be sentences, so should start with a
> capital letter and end with a period followed by two spaces.  Yes,
> we're that nit picky ;)

Strange definition of sentence you have at GNU.

> There are a few instances of this.
> 
> Samuel> +  if (trace_unwinders) {
> Samuel> +    fprintf_unfiltered (gdb_stdlog, "[ Searching for unwinder for frame ...\n");
> Samuel> +  }

Point, even the Linux style guide says you only need those if you want
to match the other body in an if/else, or are in the midst of an
if/else-if/.../else chain...

> Samuel>    for (entry = table->list; entry != NULL; entry = entry->next)
> Samuel>      {
> Samuel>        struct cleanup *old_cleanup;
> Samuel> +      const char *uname = entry->unwinder->unwinder_name;
> Samuel> +
> Samuel> +      if (trace_unwinders) {
> 
> The opening brace is misplaced here, in the GNU style it goes on the
> next line.  There are a couple instances of this.

Grrr... why did RMS have to abandon the prophets Kernighan and
Ritchie?

> Samuel> +  add_setshow_boolean_cmd ("trace-tried", class_maintenance, &trace_unwinders,
> 
> I think this could just be "set debug unwinder", rather than
> "set debug unwinder trace-tried".
>
> If you do plan to send another "debug unwinder" sub-command, maybe we
> could discuss the naming now, before the patch.

Actually, I ended up deciding the one I was thinking of wouldn't be
that useful, especially when I figured out it was the kernel link
process that was causing my actual problem, so I guess "set debug
unwinder" would be fine.