This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [mi] organize possible exec async mi oc command reasons
- From: Daniel Jacobowitz <drow at false dot org>
- To: gdb-patches at sources dot redhat dot com
- Date: Sat, 30 Apr 2005 15:32:20 -0400
- Subject: Re: [mi] organize possible exec async mi oc command reasons
- References: <20050324154602.GA10558@white> <20050324160653.GB29185@nevyn.them.org> <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