From: Sergio Durigan Junior <sergiodj@redhat.com>
To: Pedro Alves <palves@redhat.com>
Cc: GDB Patches <gdb-patches@sourceware.org>
Subject: Re: [PATCH] Assert that 'length' > 0 on infcmd.c:construct_inferior_arguments
Date: Wed, 29 Jan 2020 20:24:00 -0000 [thread overview]
Message-ID: <87ftfym2yl.fsf@redhat.com> (raw)
In-Reply-To: <9d7548f2-68e5-4b58-2fc2-9745b50e1dd4@redhat.com> (Pedro Alves's message of "Wed, 29 Jan 2020 20:13:47 +0000")
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.
Thanks,
--
Sergio
GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36
Please send encrypted e-mail if possible
http://sergiodj.net/
next prev parent reply other threads:[~2020-01-29 20:18 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-29 18:05 Sergio Durigan Junior
2020-01-29 19:20 ` Pedro Alves
2020-01-29 19:58 ` Sergio Durigan Junior
2020-01-29 20:04 ` Sergio Durigan Junior
2020-01-29 20:18 ` Pedro Alves
2020-01-29 20:24 ` Sergio Durigan Junior [this message]
2020-01-29 23:25 ` Sergio Durigan Junior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ftfym2yl.fsf@redhat.com \
--to=sergiodj@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=palves@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox