From patchwork Thu Sep 10 15:49:05 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 8624 Received: (qmail 64577 invoked by alias); 10 Sep 2015 15:49:10 -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 64554 invoked by uid 89); 10 Sep 2015 15:49:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.5 required=5.0 tests=AWL, BAYES_50, SPF_PASS autolearn=ham version=3.3.2 X-HELO: usevmg21.ericsson.net Received: from usevmg21.ericsson.net (HELO usevmg21.ericsson.net) (198.24.6.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Thu, 10 Sep 2015 15:49:08 +0000 Received: from EUSAAHC003.ericsson.se (Unknown_Domain [147.117.188.81]) by usevmg21.ericsson.net (Symantec Mail Security) with SMTP id 4B.13.26730.4DB31F55; Thu, 10 Sep 2015 10:14:12 +0200 (CEST) Received: from [142.133.110.144] (147.117.188.8) by smtp-am.internal.ericsson.com (147.117.188.83) with Microsoft SMTP Server id 14.3.248.2; Thu, 10 Sep 2015 11:49:05 -0400 Subject: Re: [PATCH] Constify variables in ada-lang.c To: Pedro Alves , , Joel Brobecker References: <1440777092-18775-1-git-send-email-simon.marchi@ericsson.com> <55F0057D.9040409@redhat.com> From: Simon Marchi Message-ID: <55F1A671.5000203@ericsson.com> Date: Thu, 10 Sep 2015 11:49:05 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 MIME-Version: 1.0 In-Reply-To: <55F0057D.9040409@redhat.com> X-IsSubscribed: yes On 15-09-09 06:10 AM, Pedro Alves wrote: > That seems obvious to me -- bound and bound_buffer point to the same > at this point. I think you should go ahead and push the patch in. > > > Maybe as a follow up patch, if we moved the "bound = bound_buffer;" > line further below, the code would be a tiny bit more obvious, as both > branches of the if will have the same pattern in their last two lines. > I'd also assign "str + k" to a temporary; all that pointer arithmetic > makes things not as clear as they could be IMO. Something like > this (rebased on top of the constification): > > --- a/gdb/ada-lang.c > +++ b/gdb/ada-lang.c > @@ -11431,22 +11431,26 @@ scan_discrim_bound (char *str, int k, struct value *dval, LONGEST * px, > char *bound; > char *pend; > struct value *bound_val; > + char *pstart; > > if (dval == NULL || str == NULL || str[k] == '\0') > return 0; > > - pend = strstr (str + k, "__"); > + pstart = str + k; > + pend = strstr (pstart, "__"); > if (pend == NULL) > { > - bound = str + k; > + bound = pstart; > k += strlen (bound); > } > else > { > - GROW_VECT (bound_buffer, bound_buffer_len, pend - (str + k) + 1); > + /* Strip __ and beyond. */ > + GROW_VECT (bound_buffer, bound_buffer_len, pend - pstart + 1); > + strncpy (bound_buffer, pstart, pend - pstart); > + bound_buffer[pend - pstart] = '\0'; > + > bound = bound_buffer; > - strncpy (bound_buffer, str + k, pend - (str + k)); > - bound[pend - (str + k)] = '\0'; > k = pend - str; > } Ok, I first pushed the original patch. I pushed this as well, inspired by your suggestion: - pstart is const - factor out (pend - pstart) as well Reg-testing with RUNTESTFLAGS="--directory=gdb.ada" shows now regression. From 30275b9ec890076ac383f7c9aefe780dfe5280ef Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Thu, 10 Sep 2015 11:40:06 -0400 Subject: [PATCH] Small refactor in ada-lang.c:scan_discrim_bound Factor out common arithmetic operations for clarity. gdb/ChangeLog: * ada-lang.c (scan_discrim_bound): Factor out arithmetic operations. --- gdb/ada-lang.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c index a514f65..d166d1c 100644 --- a/gdb/ada-lang.c +++ b/gdb/ada-lang.c @@ -11432,24 +11432,29 @@ scan_discrim_bound (const char *str, int k, struct value *dval, LONGEST * px, { static char *bound_buffer = NULL; static size_t bound_buffer_len = 0; - const char *pend, *bound; + const char *pstart, *pend, *bound; struct value *bound_val; if (dval == NULL || str == NULL || str[k] == '\0') return 0; - pend = strstr (str + k, "__"); + pstart = str + k; + pend = strstr (pstart, "__"); if (pend == NULL) { - bound = str + k; + bound = pstart; k += strlen (bound); } else { - GROW_VECT (bound_buffer, bound_buffer_len, pend - (str + k) + 1); + int len = pend - pstart; + + /* Strip __ and beyond. */ + GROW_VECT (bound_buffer, bound_buffer_len, len + 1); + strncpy (bound_buffer, pstart, len); + bound_buffer[len] = '\0'; + bound = bound_buffer; - strncpy (bound_buffer, str + k, pend - (str + k)); - bound_buffer[pend - (str + k)] = '\0'; k = pend - str; }