Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments
Commit Message
On Wednesday, January 29 2020, I wrote:
> On Wednesday, January 29 2020, Pedro Alves wrote:
>
>> 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);
>
> Congrats, this is the new challenge called "How many interactions does
> it take to get a simple patch accepted?"
>
> Sorry about this brain fart. I should slow down.
>
>> 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.
>
> OK, I'll do those things *calmly* and put your name as the author of the
> patch.
Here's the patch I ended up pushing.
c47f70e2ce7b347aadbde873aae6c2df92c42180
Thanks,
@@ -1,3 +1,9 @@
+2020-01-29 Pedro Alves <palves@redhat.com>
+ Sergio Durigan Junior <sergiodj@redhat.com>
+
+ * infcmd.c (construct_inferior_arguments): Assert that
+ 'argc' is greater than 0.
+
2020-01-29 Luis Machado <luis.machado@linaro.org>
* aarch64-tdep.c (BRK_INSN_MASK): Define to 0xffe0001f.
@@ -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 (argc > 0);
+
if (startup_with_shell)
{
#ifdef __MINGW32__