This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [PATCH v3 2/6] libdw: Add dwarf_getalt, dwarf_setalt
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Thu, 17 Apr 2014 17:17:12 +0200
- Subject: Re: [PATCH v3 2/6] libdw: Add dwarf_getalt, dwarf_setalt
Hi Florian,
Looks good, just some nitpicks.
On Tue, 2014-04-15 at 14:31 +0200, Florian Weimer wrote:
> +++ b/libdw/dwarf_getalt.c
> @@ -0,0 +1,42 @@
> +/* Create descriptor from ELF descriptor for processing file.
This isn't a great description.
> diff --git a/libdw/dwarf_setalt.c b/libdw/dwarf_setalt.c
> +/* Create descriptor from ELF descriptor for processing file.
Same here.
> +void
> +dwarf_setalt (Dwarf *main, Dwarf *alt)
> +{
> + if (main->free_alt)
> + INTUSE (dwarf_end) (main->alt_dwarf);
> + main->free_alt = false;
> + main->alt_dwarf = alt;
> +}
> +INTDEF (dwarf_setalt)
Two questions. I think either answer is fine, but it should be
explicitly said/documented.
1) Should we verify the underlying ELF files to see whether there is
a .gnu_debugaltlink with matching build_id in the alt file? If not then
we should document that the user is responsible for the sanity checking.
2) Should we allow resetting the alt file once set? If we do, like here,
we should add a warning to dwarf_getalt that dwarf_setalt might
reset/end the underlying Dwarf.
> +/* Retrieve DWARF descriptor for debugaltlink data. Returns NULL if
> + no alternate debug data has been supplied. */
> +extern Dwarf *dwarf_getalt (Dwarf *main);
> +
> +/* Provides the data referenced by the .gnu_debugaltlink section. It
> + is the responsibility of the caller to ensure that the data
> + referenced by ALT stays valid until dwarf_end (MAIN) is called. */
> +extern void dwarf_setalt (Dwarf *main, Dwarf *alt);
So whatever is decided should be documented here.
Cheers,
Mark