Re: [PATCH] Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments

[Pedro Alves <palves@redhat.com>]

On 1/29/20 7:54 PM, Sergio Durigan Junior wrote:
> On Wednesday, January 29 2020, Pedro Alves wrote:

>> Uppercase ARGC.  But putting
>>
>>  gdb_assert (argc > 0);
>>
>> at the top of the function instead as I originally suggested also works
>> for me (tried current gcc master), which seems a bit better to me, as it
>> covers both branches at once.  Did it not work for you?  This makes gcc see
>> that the loops always run at least once.
> 
> Yes, this should work.
> 
> Updated patch below.

The version you sent won't build for sure -- you're checking
length instead of argc:

 @@ -268,6 +268,11 @@ construct_inferior_arguments (int argc, char **argv)
  {
    char *result;
  
 +  /* argc should always be at least 1, but we double check this
 +     here.  This is also needed to silence -Werror-stringop
 +     warnings.  */
 +  gdb_assert (length > 0);


This is OK with that fixed, but please update the commit log, here:

 "The solution here is to explicit check that 'length' is greater than
 0.  Since we're dealing with 'argc', I think it's pretty much
 guaranteed that it's going to be at least 1."

I'd go with:

 The solution here is to assert that 'argc' is greater than
 0 on entry, which makes GCC understand that the loops always
 run at least once, and thus 'length' is always > 0.

... and also update the commit's subject.  I'd use

 Fix -Werror-stringop error on infcmd.c:construct_inferior_arguments

instead, to talk in terms of what you're fixing instead of how
you're fixing it.

Thanks,
Pedro Alves