This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
Re: [patch] Fix a regression of dwfl_core_file_report retval
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 05 Nov 2013 22:07:01 +0100
- Subject: Re: [patch] Fix a regression of dwfl_core_file_report retval
On Fri, Nov 01, 2013 at 04:41:40PM +0100, Jan Kratochvil wrote:
> Fix false match of non-build-id disk library to build-id memory library.
>
> regressed the return value of dwfl_core_file_report as now some modules may be
> successfully reported by the function will still return 0.
>
> Moreover the sniffed vs. listed logic no longer applies as all the modules are
> reported reliably, if reported at all.
> 2013-11-01 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * core-file.c (dwfl_core_file_report): Replaced variable sniffed by
> retval. Fix one forgotten LISTED increase.
Looks good, but one nitpick (feel free to ignore) and one comment.
> index 37613b8..b8087dd 100644
> --- a/libdwfl/core-file.c
> +++ b/libdwfl/core-file.c
> @@ -462,14 +462,14 @@ dwfl_core_file_report (Dwfl *dwfl, Elf *elf, const char *executable)
>
> struct r_debug_info r_debug_info;
> memset (&r_debug_info, 0, sizeof r_debug_info);
> - int listed = dwfl_link_map_report (dwfl, auxv, auxv_size,
> + int retval = dwfl_link_map_report (dwfl, auxv, auxv_size,
> dwfl_elf_phdr_memory_callback, elf,
> &r_debug_info);
> + int listed = MAX (0, retval);
Nitpick. I would find the following slightly clearer.
int listed = retval > 0 ? retval : 0;
> /* We return the number of modules we found if we found any.
> If we found none, we return -1 instead of 0 if there was an
> error rather than just nothing found. */
> - return sniffed || listed >= 0 ? listed + sniffed : listed;
> + return listed > 0 ? listed : retval;
The comment doesn't match anymore when retval == 0.
This looks simpler:
return listed > 0 ? listed : -1;
Cheers,
Mark