This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add option to control checking of inner frames when doing a backtrace
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Joshua Watt <jpewdev at gmail dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Wed, 3 Oct 2012 06:56:55 -0700
- Subject: Re: [PATCH] Add option to control checking of inner frames when doing a backtrace
- References: <CAEPrYjRZDs1JV93cWRH1JUEMcQEW2aqtZ6fS_4i9g9wuHfR1VQ@mail.gmail.com>
Hi Joshua,
> Please note: this is the first patch I have ever submitted for GDB, so
> please let me know if there are any problems. -- Joshua Watt
Thanks for sending in a patch. I suggest you read through the
gdb/CONTRIBUTE file. It should answer most of the questions.
But I still think that it's just better overall to dump the check
rather than providing yet another obscure switch which, normally,
would require us to test that it actually works as expected.
Just for your future contributions, I am including some little notes
below, that should help you conform to our project's development style
better next time... We're mostly following the GNU Coding Style with
some project-specific additions.
And one last thing: New commands and settings should be documented
in the gdb/NEWS files as well as the GDB manual (gdb/doc/gdb.texinfo).
========================================================================
Aside from missing a ChangeLog entry, GDB's coding style also
require that every global variable and function be documented.
For instance:
> +static unsigned int backtrace_check_inner = TRUE;
There should be a comment before backtrace_check_inner's definition:
/* Bla bla bla. */
static unsigned int backtrace_check_inner = TRUE;
And while looking at this, we typically use int, and zero/nonzero
values for this type of usage. If you look at add_setshow_boolean_cmd,
you'll see that the value passed is an int*.
Similarly:
> +static void
> +show_backtrace_check_inner (struct ui_file *file, int from_tty,
> + struct cmd_list_element *c, const char *value)
And there should also be a comment before show_backtrace_check_inner.
For functions, we add an empty line between documentation and function.
Another small nit: The last line needs to be aligned with the rest
of the arguments (using tabs, and then optionally spaces):
show_backtrace_check_inner (struct ui_file *file, int from_tty,
struct cmd_list_element *c, const char *value)
--
Joel