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

[Pedro Alves <palves at redhat dot com>]

On 08/28/2015 04:51 PM, Simon Marchi wrote:
> I found this const/not const mixup found by building in C++ mode.
> 
> I would push this as obvious, but I am not sure I understand what
> happens in scan_discrim_bound, so I'd like it to be reviewed.
> 


> @@ -11446,11 +11445,12 @@ scan_discrim_bound (char *str, int k, struct value *dval, LONGEST * px,
>        GROW_VECT (bound_buffer, bound_buffer_len, pend - (str + k) + 1);
>        bound = bound_buffer;
>        strncpy (bound_buffer, str + k, pend - (str + k));
> -      bound[pend - (str + k)] = '\0';
> +      bound_buffer[pend - (str + k)] = '\0';
>        k = pend - str;
>      }

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;
     }


Thanks,
Pedro Alves