Re: [mi] organize possible exec async mi oc command reasons

Daniel Jacobowitz
2005-04-30T19:32:27Z

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