From patchwork Wed Feb 4 17:19:44 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Wielaard X-Patchwork-Id: 4912 Received: (qmail 3345 invoked by alias); 4 Feb 2015 17:19:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 3331 invoked by uid 89); 4 Feb 2015 17:19:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.4 required=5.0 tests=AWL, BAYES_00, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 04 Feb 2015 17:19:50 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t14HJkok003424 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Wed, 4 Feb 2015 12:19:46 -0500 Received: from bordewijk.wildebeest.org (ovpn-116-48.ams2.redhat.com [10.36.116.48]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t14HJjbb008234 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Wed, 4 Feb 2015 12:19:46 -0500 Received: by bordewijk.wildebeest.org (Postfix, from userid 1000) id 98D0A8148039; Wed, 4 Feb 2015 18:19:44 +0100 (CET) Message-ID: <1423070384.4947.47.camel@bordewijk.wildebeest.org> Subject: Re: [PATCH] Merge GCC producer parsers. Allow digits in identifiers. From: Mark Wielaard To: Joel Brobecker Cc: gdb-patches@sourceware.org Date: Wed, 04 Feb 2015 18:19:44 +0100 In-Reply-To: <1422535409.4947.5.camel@bordewijk.wildebeest.org> References: <1422185341-20243-1-git-send-email-mjw@redhat.com> <20150129080018.GH5193@adacore.com> <1422535409.4947.5.camel@bordewijk.wildebeest.org> Mime-Version: 1.0 On Thu, 2015-01-29 at 13:43 +0100, Mark Wielaard wrote: > On Thu, 2015-01-29 at 12:00 +0400, Joel Brobecker wrote: > > Looking at the function, it seems to me that the current design of > > the function is a little strange: Named "producer_is_gcc", it'd expect > > the return value to be a boolean. So, if the function returned that > > and had an extra parameter for getting the major, the issue would > > not arise: > > > > else if (producer_is_gcc (cu->producer, &major, &minor)) > > > > I think you chose this approach out of consistency with > > producer_is_gcc_ge_4, which is why I'm OK with the current patch. > > But if I were to look into this, what I would do replace the two > > calls to this function with calls to your new function. > > > > I might also allow major/minor to be NULL to spare users the need > > to create variables that they will not need afterwards. > > > > So, to sumarize, push as is, and then let's clean this up (you don't > > have to do the cleanup, if you don't want to, but let me know if you > > pass, as I'd like to take over if you do). > > Thanks, I pushed as is. You observations are correct. > I'll refactor this as you suggest in a separate patch. How about the following cleanup: Change producer_is_gcc function return type to bool. gdb/ChangeLog: * utils.h (producer_is_gcc): Change return type to bool. Add major argument. * utils.c (producer_is_gcc): Likewise. (producer_is_gcc_ge_4): Adjust producer_is_gcc call. * dwarf2read.c (check_producer): Likewise. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 0d8026f..eb8541f 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -12293,7 +12293,7 @@ check_producer (struct dwarf2_cu *cu) combination. gcc-4.5.x -gdwarf-4 binaries have DW_AT_accessibility interpreted incorrectly by GDB now - GCC PR debug/48229. */ } - else if ((major = producer_is_gcc (cu->producer, &minor)) > 0) + else if (producer_is_gcc (cu->producer, &major, &minor)) { cu->producer_is_gxx_lt_4_6 = major < 4 || (major == 4 && minor < 6); cu->producer_is_gcc_lt_4_3 = major < 4 || (major == 4 && minor < 3); diff --git a/gdb/utils.c b/gdb/utils.c index 909476b..99b3874 100644 --- a/gdb/utils.c +++ b/gdb/utils.c @@ -3259,7 +3259,8 @@ int producer_is_gcc_ge_4 (const char *producer) { int major, minor; - major = producer_is_gcc (producer, &minor); + if (! producer_is_gcc (producer, &major, &minor)) + return -1; if (major < 4) return -1; if (major > 4) @@ -3267,17 +3268,22 @@ producer_is_gcc_ge_4 (const char *producer) return minor; } -/* Returns the major version number if the given PRODUCER string is GCC and - sets the MINOR version. Returns -1 if the given PRODUCER is NULL or it - isn't GCC. */ -int -producer_is_gcc (const char *producer, int *minor) +/* Returns true if the given PRODUCER string is GCC and sets the MAJOR + and MINOR versions when not NULL. Returns false if the given PRODUCER + is NULL or it isn't GCC. */ +bool +producer_is_gcc (const char *producer, int *major, int *minor) { const char *cs; - int major; if (producer != NULL && strncmp (producer, "GNU ", strlen ("GNU ")) == 0) { + int maj, min; + if (major == NULL) + major = &maj; + if (minor == NULL) + minor = &min; + /* Skip any identifier after "GNU " - such as "C11" "C++" or "Java". A full producer string might look like: "GNU C 4.7.2" @@ -3289,7 +3295,7 @@ producer_is_gcc (const char *producer, int *minor) cs++; if (*cs && isspace (*cs)) cs++; - if (sscanf (cs, "%d.%d", &major, minor) == 2) + if (sscanf (cs, "%d.%d", major, minor) == 2) return major; } diff --git a/gdb/utils.h b/gdb/utils.h index 6724d7c..d8afa79 100644 --- a/gdb/utils.h +++ b/gdb/utils.h @@ -21,6 +21,8 @@ #ifndef UTILS_H #define UTILS_H +#include + #include "exceptions.h" extern void initialize_utils (void); @@ -302,7 +304,7 @@ extern pid_t wait_to_die_with_timeout (pid_t pid, int *status, int timeout); #endif extern int producer_is_gcc_ge_4 (const char *producer); -extern int producer_is_gcc (const char *producer, int *minor); +extern bool producer_is_gcc (const char *producer, int *major, int *minor); extern int myread (int, char *, int);