From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26560 invoked by alias); 14 Jan 2011 16:30:44 -0000 Received: (qmail 26551 invoked by uid 22791); 14 Jan 2011 16:30:42 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 14 Jan 2011 16:30:38 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 5479C2BAC69; Fri, 14 Jan 2011 11:30:36 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id D4nUYTRNjrXI; Fri, 14 Jan 2011 11:30:36 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 41B8F2BAB32; Fri, 14 Jan 2011 11:30:36 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id A7C9C1459AD; Fri, 14 Jan 2011 11:30:35 -0500 (EST) Date: Fri, 14 Jan 2011 16:32:00 -0000 From: Joel Brobecker To: Pierre Muller Cc: gdb-patches@sourceware.org Subject: Re: [RFC] Fix hpux_major_release variable setting Message-ID: <20110114163035.GQ2504@adacore.com> References: <000f01cbb401$1093cdc0$31bb6940$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <000f01cbb401$1093cdc0$31bb6940$@muller@ics-cnrs.unistra.fr> User-Agent: Mutt/1.5.20 (2009-06-14) Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-01/txt/msg00330.txt.bz2 > The patch below fixes that problem by moving the call to uname to > hppa-hpux-nat.c > > Is this patch OK? I think that for a platform such as pa-hpux, this is fine (MarkK, who just informed us that he has access to HP-UX hardware as well, might want to coment as well). But I've since learnt that the correct way of doing this is to use target_xfer_partial routine. The reason why I think it's OK this way is that the fix seems contains to pa/hp-ux and I don't see a lot of benefit in spending time and energy into trying to support HP-UX 10.x. > I am unsure about the way to move it to the header... Would it be > better to declare it without external in the header? I don't think that this would work. You'd end up with multiple definitions of the same global variable. > This would then mean that I should also move > DEFAULT_HPUX_MAJOR_RELEASE macro to the header, but it would have > the advantage of allowing to not use hard-coded 11 in > _initialize_hppa_hpux_nat. You can just leave the variable untouched instead of setting it to 11... > 2011-01-14 Pierre Muller > > * solib-som.h (hpux_major_release): Declare variable here. > * solib-som.c: Remove header. > (DEFAULT_HPUX_MAJOR_RELEASE): New macro. > (hpux_major_release): Make global, change default value to > DEFAULT_HPUX_MAJOR_RELEASE. > (get_hpux_major_release): Simply return HPUX_MAJOR_RELEASE. > * hppa-hpux-nat.c: Add include. > Add "solib-som.h" header. > (_initialize_hppa_hpux_nat): Set hpux_major_release variable by > analyzis of uname call return. > +/* Variable storing HP-UX major release number. > + On non native system, simply suppose that the major release number is ^^^^^^^^ assume > 11. > + hppa-hpux-nat.c initialization code sets this number to > + the real one on startup in native conditions. */ Suggest the following formatting (following the 70 characters guideline, as well as an empty line for better clarity (personal opinion, can be ignored). I'd also add a comment explaining why this value is computed elsewhere. So here is a suggested rewordin: /* Variable storing HP-UX major release number. On non-native system, simply assume that the major release number is 11. On native systems, hppa-hpux-nat.c initialization code sets this number to the real one on startup in native conditions. We cannot compute this value here, because we need to make a native call to "uname". We are are not allowed to do that from here, as this file is used for both native and cross debugging. */ > +#define DEFAULT_HPUX_MAJOR_RELEASE 11 > +int > +hpux_major_release = DEFAULT_HPUX_MAJOR_RELEASE; Formatting: int hpux_major_release = DEFAULT_HPUX_MAJOR_RELEASE; (I'm wondering if the DEFAULT_HPUX_MAJOR_RELEASE macro really brings much). > static int > get_hpux_major_release (void) Let's get rid of this function at the same time, WDYT? It should be a simple search-and-replace... > + struct utsname x; > + char *p; > > + uname (&x); > + p = strchr (x.release, '.'); > + hpux_major_release = p ? atoi (p + 1) : 11; Let's make that a function, and call the function from the initialization code. I think it'll be easier to understand. Otherwise, a small comment, and a new-line after the code block would help visually separate this operation from the rest. -- Joel