From: Daniel Jacobowitz <drow@false.org>
To: gdb-patches@sources.redhat.com
Subject: Re: [mi] organize possible exec async mi oc command reasons
Date: Sat, 30 Apr 2005 19:32:00 -0000 [thread overview]
Message-ID: <20050430193220.GG7009@nevyn.them.org> (raw)
In-Reply-To: <20050324212036.GB10808@white>
On Thu, Mar 24, 2005 at 04:20:36PM -0500, Bob Rossi wrote:
> > I don't have much comment about the patch; it seems like a plausible
> > idea. However, please pay attention to the coding and formatting
> > standards:
> >
> > - ChangeLog entries are capitalized and end with periods.
> > - Function definitions have the function name in the first column.
> > - Function braces are in the first column.
> > - There's a space before function argument lists.
> > - Function argument lists have to be indented normally; an argument
> > on a new line goes at the same depth as the first argument on the
> > previous line.
> > - Comments don't use multiple leading *s.
> >
> > and so forth.
> >
> > I sometimes consider myself the pedantic guardian of GDB's source code
> > formatting :-)
>
> Very sorry for not getting the formatting correct. I respect the rules,
> I just have a hard time formatting everything perfectly, I'm sure I can
> do better!
You're still having a bit of trouble with those pesky spaces before
function argument lists. You've also got spaces in the first column;
please don't start lines with eight spaces, make sure you use tabs
appropriately instead.
For example:
ui_out_field_string (uiout, "reason",
async_reason_lookup(EXEC_ASYNC_BREAKPOINT_HIT));
Needs to be:
ui_out_field_string (uiout, "reason",
async_reason_lookup (EXEC_ASYNC_BREAKPOINT_HIT));
When you wrap a line between a function and its argument list, it gains
one level of indentation (i.e. two spaces).
> static const char *async_reason_string_lookup[EXEC_ASYNC_LAST+1] =
> {
> "breakpoint-hit",
> "watchpoint-trigger",
> "read-watchpoint-trigger",
> "access-watchpoint-trigger",
> "function-finished",
> "location-reached",
> "watchpoint-scope",
> "end-stepping-range",
> "exited-signalled",
> "exited",
> "exited-normally",
> "signal-received",
> (char*)0
NULL, please.
A couple of other places in GDB have similar idioms, for instance
osabi.c/osabi.h. If you use [] for the array, you can create an
_initialize_mi_common function which verifies that the array has the
correct number of entries and issues an internal error if they get out
of step.
You can also do this as a compile time test in C, but it's kind of
ugly. For instance, here's one way to do it:
static int dummy[(ARRAY_SIZE (async_reason_string_lookup)
== EXEC_ASYNC_LAST + 1) ? 0 : -1];
Probably should stick with the internal error; that way there's an
obvious message associated.
The accessor function should issue an internal error if the reason is
out of bounds.
The documentation had a couple of comma-separated lists with missing
spaces, i.e. "-exec-step,-exec-next" should be "-exec-step, -exec-next".
Would you mind reposting the patch with these fixes, and the latest
revision of the documentation?
--
Daniel Jacobowitz
CodeSourcery, LLC
next prev parent reply other threads:[~2005-04-30 19:32 UTC|newest]
Thread overview: 29+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-03-24 15:46 Bob Rossi
2005-03-24 15:54 ` Daniel Jacobowitz
2005-03-24 16:06 ` Daniel Jacobowitz
2005-03-24 18:47 ` Stan Shebs
2005-03-24 21:20 ` Bob Rossi
2005-03-26 10:43 ` Eli Zaretskii
2005-04-30 19:32 ` Daniel Jacobowitz [this message]
2005-05-18 3:29 ` Bob Rossi
2005-05-18 3:34 ` Daniel Jacobowitz
2005-05-18 3:36 ` Bob Rossi
2005-05-18 3:43 ` Daniel Jacobowitz
2005-05-18 4:00 ` Bob Rossi
2005-05-18 8:53 ` Bob Rossi
2005-05-26 2:35 ` Bob Rossi
2005-05-28 18:56 ` Daniel Jacobowitz
2005-05-29 2:55 ` Bob Rossi
2005-05-29 3:00 ` Bob Rossi
2005-05-29 4:57 ` Daniel Jacobowitz
2005-05-29 5:53 ` Bob Rossi
2005-05-29 3:11 ` Daniel Jacobowitz
2005-06-15 19:25 ` David Lecomber
2005-06-15 20:03 ` Bob Rossi
2005-06-15 20:46 ` Daniel Jacobowitz
2005-03-24 20:00 ` Eli Zaretskii
2005-03-24 21:02 ` Bob Rossi
2005-03-26 10:40 ` Eli Zaretskii
2005-03-26 13:32 ` Eli Zaretskii
2005-03-27 2:56 ` Bob Rossi
2005-03-27 4:42 ` Eli Zaretskii
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=20050430193220.GG7009@nevyn.them.org \
--to=drow@false.org \
--cc=gdb-patches@sources.redhat.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