Re: [PATCH] Constify variables in ada-lang.c

[Simon Marchi <simon dot marchi at ericsson dot com>]

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 <simon.marchi@ericsson.com>
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;
     }

-- 
2.1.4