This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]