Hi Aaron,
On Thu, Jan 30, 2025 at 09:35:53PM -0500, Aaron Merey wrote:
> test-elf_cntl_gelf_getshdr conditionally closes a file descriptor
> depending on a command line argument. This causes an error when run
> under valgrind --track-fds=yes.
>
> Fix this by unconditionally closing the fd.
I think this subtly changes what is being tested here. If I remember
correctly this tests that gelf_getshdr works correctly even when the
underlying fd is closed and the file is either mmapped or elf_cntl
with ELF_C_FDREAD is called.
The idea being that if the file is mmapped then everything needed for
reading the ELF file is already in memory. And when elf_cntl is called
with ELF_C_FDREAD then libelf must make sure the same is true.
You could argue that the test should explicitly call elf_cntl with
ELF_C_FDDONE after the close (fd) though.
So I don't think close should be called unconditionally. Maybe the
correct solution is to do if (!close_fd) close (fd); after elf_end?
Cheers,
Mark
> Signed-off-by: Aaron Merey <ame...@redhat.com>
> ---
> tests/test-elf_cntl_gelf_getshdr.c | 20 +++++---------------
> 1 file changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/tests/test-elf_cntl_gelf_getshdr.c
> b/tests/test-elf_cntl_gelf_getshdr.c
> index 7371110c..9f78bec2 100644
> --- a/tests/test-elf_cntl_gelf_getshdr.c
> +++ b/tests/test-elf_cntl_gelf_getshdr.c
> @@ -43,22 +43,12 @@ main (int argc, char *argv[])
> }
>
> bool do_mmap = false;
> - bool close_fd = false;
> if (strcmp (argv[1], "READ") == 0)
> - {
> - do_mmap = false;
> - close_fd = false;
> - }
> + do_mmap = false;
> else if (strcmp (argv[1], "MMAP") == 0)
> - {
> - do_mmap = true;
> - close_fd = false;
> - }
> + do_mmap = true;
> else if (strcmp (argv[1], "FDREAD") == 0)
> - {
> - do_mmap = false;
> - close_fd = true;
> - }
> + do_mmap = false;
> else
> {
> fprintf (stderr, "First arg needs to be 'READ', 'MMAP' or 'FDREAD'\n");
> @@ -83,7 +73,7 @@ main (int argc, char *argv[])
> exit (2);
> }
>
> - if (! do_mmap && close_fd)
> + if (! do_mmap)
> {
> if (elf_cntl (elf, ELF_C_FDREAD) < 0)
> {
> @@ -91,7 +81,6 @@ main (int argc, char *argv[])
> elf_errmsg (-1));
> exit (1);
> }
> - close (fd);
> }
>
> Elf_Scn *scn = NULL;
> @@ -103,5 +92,6 @@ main (int argc, char *argv[])
> }
>
> elf_end (elf);
> + close (fd);
> exit (0);
> }
> --
> 2.48.1
>
