On 04/10/2014 06:10 PM, Mark Wielaard wrote:
Hi Florian,
On Thu, 2014-04-10 at 13:19 +0200, Florian Weimer wrote:
+2014-04-10 Florian Weimer <[email protected]>
+
+ * libdw.h (dwarf_debugaltlink): New function.
+ * libdwP.h (IDX_debug_info): Add IDX_gnu_debugaltlink.
+ (dwarf_debugaltlink): Internal declaration.
+ * Makefile.am (libdw_a_SOURCES): Add dwarf_debugaltlink.c.
+ * libdw.map (ELFUTILS_0.159): Export dwarf_debugaltlink.
+ * dwarf_debugaltlink.c: New file.
+ * dwarf_begin_elf.c (dwarf_scnnames): Add IDX_gnu_debugaltlink
+ element.
+ (check_section): Call open_debugaltlink.
I like the helper function, but I think it is a little more low-level
than what we normally provide. We should at least (also) have a
dwarf_get_alt () that returns a Dwarf *.
This would just return the stored Dwarf *, and not attempt to open any
files?
If we provide this low-level function then it would be good to also
provide a similar one for .gnu_debuglink (which already exists privately
as find_debuglink in libdwfl) and for the build_id of an Elf file (which
also exist privately as __libdwfl_find_elf_build_id in libdwfl, but that
does a bit more by also determining the load address). If we provide
those helper functions because they might indeed be useful to people
wanting to write their own Elf/Dwarf file search functions, I am not
sure how to call them. I don't want people to confuse these lower-level
helper functions (which expose Elf file details) with the higher-level
libdw/Dwarf functionality.
We could add them at the ELF layer, but then we'd have to iterate over
all the ELF section headers in each of these functions. The section
header array in libdw avoids this. I'm also not sure where to put such
convenience functions. libebl? But I currently don't use that, and I
think some downstreams assign send-class status to it.
+int dwarf_debugaltlink (Dwarf *dwarf,
+ const char **alt_name,
+ const void **build_id,
+ size_t *build_id_len)
+{
+ Elf_Data *data = dwarf->sectiondata[IDX_gnu_debugaltlink];
+ if (data == NULL)
+ {
+ __libdw_seterrno (DWARF_E_NO_ALT_DEBUGLINK);
+ return -1;
+ }
+
+ const void *ptr = memchr (data->d_buf, '\0', data->d_size);
+ if (ptr == NULL)
+ {
+ __libdw_seterrno (DWARF_E_INVALID_ELF);
+ return -1;
+ }
+ *alt_name = data->d_buf;
+ *build_id = ptr + 1;
+ *build_id_len = data->d_size - (ptr - data->d_buf + 1);
+ return 0;
+}
I would make build_id_len the return value. Then you can return:
- 0 No debugaltlink found.
- > 0 Length of build_id, success.
- < 0 Error.
Hmm, I wonder if the interface is similar enough to read(2) so that the
tri-state return value isn't a problem. Usually, it's problematic
because callers are tempted to treat the result as boolean, interpreting
-1 as success.
I've picked up the other suggestions.
--
Florian Weimer / Red Hat Product Security Team
