From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7865 invoked by alias); 30 Apr 2005 19:32:27 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 7811 invoked from network); 30 Apr 2005 19:32:21 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sourceware.org with SMTP; 30 Apr 2005 19:32:21 -0000 Received: from drow by nevyn.them.org with local (Exim 4.50 #1 (Debian)) id 1DRxhM-0001k2-Vs for ; Sat, 30 Apr 2005 15:32:21 -0400 Date: Sat, 30 Apr 2005 19:32:00 -0000 From: Daniel Jacobowitz To: gdb-patches@sources.redhat.com Subject: Re: [mi] organize possible exec async mi oc command reasons Message-ID: <20050430193220.GG7009@nevyn.them.org> Mail-Followup-To: gdb-patches@sources.redhat.com References: <20050324154602.GA10558@white> <20050324160653.GB29185@nevyn.them.org> <20050324212036.GB10808@white> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050324212036.GB10808@white> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-04/txt/msg00455.txt.bz2 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