This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink
- From: Florian Weimer <fweimer at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 22 Apr 2014 09:33:35 +0200
- Subject: Re: [PATCH v3 3/6] libdwelf: Add dwelf_dwarf_gnu_debugaltlink
On 04/18/2014 01:02 PM, Mark Wielaard wrote:
> On Tue, 2014-04-15 at 14:49 +0200, Florian Weimer wrote:
>> +2014-04-15 Florian Weimer <fweimer@redhat.com>
>> +
>> + * dwelf_dwarf_gnu_debugaltlink.c: New file.
>> + * Makefile.am (libdwelf_a_SOURCES): Add it.
>> + * libdwelf.h (dwelf_dwarf_gnu_debugaltlink): Declare new function.
>> + * libdwelfP.h (dwelf_dwarf_gnu_debugaltlink): Add internal
>> + declaration.
>
> The code looks fine. But I still like the "tri-state" return more to
> indicate why/how things went. Precisely because then you don't have to
> do:
>
>> + Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
>> + if (data == NULL)
>> + {
>> + /* Missing data is not actually an error. */
>> + __libdw_seterrno (0);
>> + return NULL;
>> + }
>> [...]
>> +/* Returns the name of the alternate debug information file from the
>> + .gnu_debugaltlink section if found in the ELF. On success, writes
>> + a pointer to the build ID to *BUILD_IDP, and its length to
>> + *BUILD_ID_LENP. Returns NULL if the ELF file didn't have a
>> + .gnu_debuglink section (and sets the dwarf_errcode error code to
>> + 0), had malformed data in the section or some other error occured.
>> + The returned pointers are valid while the ELF handle is valid. */
>> +extern const char *dwelf_dwarf_gnu_debugaltlink (Dwarf *dwarf,
>> + const void **build_idp,
>> + size_t *build_id_lenp);
I'm not sure if this addresses this completely addresses the issue. A
return value of 0 could mean "no debugaltlink data" or "the build ID has
zero length".
I don't like setting the error code to zero, but as far as I can tell,
the error codes are not exposed, and a caller would have to resort to
string comparisons to check if it's the expected "missing section"
error, or something about lack of resources or malformed input data.
>> +2014-04-15 Florian Weimer <fweimer@redhat.com>
>> +
>> + * debugaltlink.c, run-debugaltlink.sh: New files.
>> + * Makefile.am (check_PROGRAMS): Add debugaltlink.
>> + (TESTS): Add run-debugaltlink.sh.
>> + (debugaltlink_LDADD): New variable.
>
> Thanks for adding the testcase. run-debugaltlink.sh should also be added
> to EXTRA_DIST (I also forgot that in my dwelf_elf_gnu_debuglink patch
> (fixed on the mjw/dwelf branch).
Can't we handle this in a better way with make variables, so that the
file needs to be listed in just one place?
Anyway, I've fixed it in my copy.
--
Florian Weimer / Red Hat Product Security Team