Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
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


  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