From: Joel Brobecker <brobecker@adacore.com>
To: Joshua Watt <jpewdev@gmail.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH] Add option to control checking of inner frames when doing a backtrace
Date: Wed, 03 Oct 2012 13:57:00 -0000 [thread overview]
Message-ID: <20121003135655.GF3028@adacore.com> (raw)
In-Reply-To: <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
next prev parent reply other threads:[~2012-10-03 13:57 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-02 17:58 Joshua Watt
2012-10-03 9:29 ` Abid, Hafiz
2012-10-03 14:03 ` Joshua Watt
2012-10-03 13:57 ` Joel Brobecker [this message]
2012-10-05 21:10 ` Joshua Watt
2012-10-06 18:34 ` Jan Kratochvil
2012-10-07 5:52 ` Jan Kratochvil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121003135655.GF3028@adacore.com \
--to=brobecker@adacore.com \
--cc=gdb-patches@sourceware.org \
--cc=jpewdev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox