[PATCH 4/5] tests/test-elf_cntl_gelf_getshdr.c: Close fd unconditionally

[Mark Wielaard]

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 <amerey@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
>