Re: [COMMITTED] debuginfod_find_section: Always update rc with most recent error code

[Mark Wielaard <mark@klomp.org>]

Hi Aaron,

On Fri, 2022-11-04 at 17:40 -0400, Aaron Merey via Elfutils-devel
wrote:
> debuginfod_find_section may attempt to download both the debuginfo
> and executable matching the given build-id.  If neither of these
> files can be found, update rc to ensure that we always return an
> accurate error code in this case.

Nicely spotted, otherwise we might have returned -EEXIST.

> diff --git a/debuginfod/debuginfod-client.c b/debuginfod/debuginfod-
> client.c
> index f48e32cc..99da05ef 100644
> --- a/debuginfod/debuginfod-client.c
> +++ b/debuginfod/debuginfod-client.c
> @@ -1944,7 +1944,8 @@ debuginfod_find_section (debuginfod_client *client,
>  
>    if (rc == -EEXIST)
>      {
> -      /* The section should be found in the executable.  */
> +      /* Either the debuginfo couldn't be found or the section should
> +	 be in the executable.  */
>        fd = debuginfod_find_executable (client, build_id,
>  				       build_id_len, &tmp_path);
>        if (fd > 0)

I know this is in existing code, so this might have missed in a
previous review. But shouldn't this be fd >= 0 ?

That is what is checked in the rest of the code. Except for the
debuginfod_find_section function which uses fd >0 twice.

It is unlikely, but I think fd can be zero if it (stdin) was closed by
the program for some reason. Then I think zero can be reused as new
file descriptor?

Cheers,

Mark