This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [RFA 5/7 take 4] Improved linker-debugger interface
- From: Pedro Alves <palves at redhat dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>, gdb-patches at sourceware dot org
- Date: Mon, 03 Jun 2013 11:31:13 +0100
- Subject: Re: [RFA 5/7 take 4] Improved linker-debugger interface
- References: <20130516144340 dot GA2105 at blade dot nx> <20130516144838 dot GF2105 at blade dot nx> <20130517185002 dot GA10447 at host2 dot jankratochvil dot net> <20130524083044 dot GC4602 at blade dot nx> <51A64E24 dot 2020901 at redhat dot com> <20130530104346 dot GA6217 at blade dot nx> <51A789CF dot 2000302 at redhat dot com> <20130531132206 dot GB7409 at blade dot nx>
On 05/31/2013 02:22 PM, Gary Benson wrote:
> Pedro Alves wrote:
>> On 05/30/2013 11:43 AM, Gary Benson wrote:
>>> + char name[32] = { '\0' };
>>> +
>>> + /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4
>>> + shipped with an early version of the probes code in
>>> + which the probes' names were prefixed with "rtld_"
>>> + and the "map_failed" probe did not exist. The
>>> + locations of the probes are otherwise the same, so
>>> + we check for probes with prefixed names if probes
>>> + with unprefixed names are not present. */
>>> +
>>> + if (with_prefix)
>>> + strncat (name, "rtld_", sizeof (name) - strlen (name) - 1);
>>> +
>>> + strncat (name, probe_info[i].name,
>>> + sizeof (name) - strlen (name) - 1);
>>
>> BTW, I'd rather this was written as something like:
>>
>> char name[32];
>>
>> if (with_prefix)
>> xsnprintf (name, sizeof (name), "rtld_%s", probe_info[i].name);
>> else
>> xsnprintf (name, sizeof (name), "%s", probe_info[i].name);
>>
>>
>> strncat like that is really not future proof, as we'll just get get
>> a silently truncated string if a new probe appears with a name that
>> is too big. xsnprintf OTOH gdb_asserts the string fits in the
>> buffer. Granted, whoever added such a probe would probably notice
>> this, but it's a style best avoided. (Witness how so few strncat
>> calls there are in gdb. There are more strcat calls, but that only
>> shows that whenever we _do_ care about string overruns, we tend to
>> use something else, not strncat.)
>> Plus, it looks more readable to me that way. :-)
>> Alternatively, we could add a xstrncat that asserts truncation
>> never happens, though xsnprintf in this case still looks a bit
>> more readable to me :-)
>
> How about this:
>
> const char *name = probe_info[i].name;
> char buf[32];
>
> /* Fedora 17 and Red Hat Enterprise Linux 6.2-6.4
> shipped with an early version of the probes code in
> which the probes' names were prefixed with "rtld_"
> and the "map_failed" probe did not exist. The
> locations of the probes are otherwise the same, so
> we check for probes with prefixed names if probes
> with unprefixed names are not present. */
> if (with_prefix)
> {
> xsnprintf (buf, sizeof (buf), "rtld_%s", name);
> name = buf;
> }
>
> Revised patch attached.
That's great, thanks.
--
Pedro Alves