[PATCH v3 REVIEW 06/33] binutils: readelf: when dumping CTF, load strtab and symtab automatically

[Nick Alcock]

On 9 Sep 2019, Alan Modra outgrape:

> On Fri, Sep 06, 2019 at 11:54:53PM +0100, Nick Alcock wrote:
>> --- a/binutils/readelf.c
>> +++ b/binutils/readelf.c
>> @@ -13944,7 +13944,13 @@ dump_section_as_ctf (Elf_Internal_Shdr * section, Filedata * filedata)
>>    data = get_section_contents (section, filedata);
>>    ctfsect.cts_data = data;
>>  
>> -  if (dump_ctf_symtab_name)
>> +  if (!dump_ctf_symtab_name)
>> +    dump_ctf_symtab_name = strdup (".symtab");
>
> Please use xstrdup here and elsewhere, fixing the other uses of strdup
> you added in a previous patch.

I actually used strdup here because readelf had *no* uses of xstrdup in
it, but did use strdup, making me think that perhaps there was some
reason I wasn't getting why strdup was preferred here.

I'll flip them all.

(I can't see any previous-patch uses of strdup. Recently introduced uses
in libctf are divided into two classes: ctf_type_aname_raw, which
specifically wants to return a newly-dupped string or NULL, so strdup is
right, and the ctf-link machinery, which is *not* intended purely for
use from ld and so should use good library philosophy and return errors
rather than dying. However, I have been too cavalier in a few places
there and failed to check strdup() for errors well enough: will fix.)

> Note that the alternative of testing the result and printing an error
> message is more suited to library code where you want to leave the
> application some means of recovery.

Quite so, which is why I feel guilty whenever I cop out and use xstrdup
in libctf :) it does happen, though less often now we have the atoms
table to hang on to strings persistently.

I think I'll decommission ctf_strdup soon: it's getting in the way of
audits like this. ctf_alloc too -- I've been hoping it might be useful
but it's proving to be pure obfuscation and not helpful at all. If I
want to interpose malloc for debugging, there are better ways.

-- 
NULL && (void)