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