From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8296 invoked by alias); 2 May 2005 04:05:44 -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 8277 invoked from network); 2 May 2005 04:05:40 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sourceware.org with SMTP; 2 May 2005 04:05:40 -0000 Received: from drow by nevyn.them.org with local (Exim 4.50 #1 (Debian)) id 1DSSBS-0002cb-8v; Mon, 02 May 2005 00:05:26 -0400 Date: Mon, 02 May 2005 04:05:00 -0000 From: Daniel Jacobowitz To: Nick Roberts Cc: Bob Rossi , Andrew Cagney , gdb-patches@sources.redhat.com Subject: Re: [PATCH: gdb/mi + doco] -var-update Message-ID: <20050502040526.GA10023@nevyn.them.org> Mail-Followup-To: Nick Roberts , Bob Rossi , Andrew Cagney , gdb-patches@sources.redhat.com References: <16919.53411.753668.336933@farnswood.snap.net.nz> <01c51709$Blat.v2.4$4a3292a0@zahav.net.il> <16921.18627.457594.938060@farnswood.snap.net.nz> <01c517d0$Blat.v2.4$09a26040@zahav.net.il> <16922.43915.346792.973282@farnswood.snap.net.nz> <01c51898$Blat.v2.4$f6fd05c0@zahav.net.il> <16929.8147.933720.246602@farnswood.snap.net.nz> <16955.41017.161288.832646@farnswood.snap.net.nz> <20050401024942.GA2179@white> <17013.35649.62745.226730@farnswood.snap.net.nz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <17013.35649.62745.226730@farnswood.snap.net.nz> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-05/txt/msg00037.txt.bz2 On Mon, May 02, 2005 at 02:06:57PM +1200, Nick Roberts wrote: > > I have some spare time waiting for my patches to get reviewed, so I > > figure'd I'd look at yours. If you care, I have just a few comments. > > Sorry, Bob. I read this too quickly the first time and (stupidly) thought > that you had forgot to include the comments. > > > > + const char novalues[] = "\"--no-values\""; > > > + const char withvalues[] = "\"--with-values\""; > > > + const char simplevalues[] = "\"--simple-values\""; > > > + const char allvalues[] = "\"--all-values\""; > > > > These could be made static. > > Not really. I use them in my patch for mi-cmd-stack.c which I included in my > earlier submission (Sun, 27 Feb 2005 14:18:11 +1300) but left out on (Mar 19) > as it was unchanged. If you want these to be global variables, you need to give them better names. Also, the GDB coding style means that multiple words get_underscores_for_separation. > ... > > > ! if (strcmp (argv[0], "0") == 0 > > > ! || strcmp (argv[0], "--no-values") == 0) > > > ! print_values = PRINT_NO_VALUES; > > > ! else if (strcmp (argv[0], "1") == 0 > > > ! || strcmp (argv[0], "--with-values") == 0) > > > > instead of using "--no-values" and "--with-values" you could use the > > variable "novalues" and "withvalues". > > Yes. That would make sense. You'll need to strip the extra quote marks off those variables then - which seems more natural anyway. > I will send the revised source (all files apart from doco which Eli has > already approved) to Daniel and cc gdb-patches. Please include the documentation with the patch anyway; a complete patch lets reviewers see the whole picture more easily. -- Daniel Jacobowitz CodeSourcery, LLC