[PATCH 1/2] tst-faccessat.c: Port to libsupport

Adhemerval Zanella Netto adhemerval.zanella@linaro.org
Wed Nov 6 15:08:13 GMT 2024



On 16/10/24 16:21, Siddhesh Poyarekar wrote:
> Use libsupport convenience functions and macros instead of the old
> test-skeleton.  Also add a new xdup() convenience wrapper function.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

LGTM, some minor comments below.

> ---
>  io/tst-faccessat.c | 219 +++++++++++++--------------------------------
>  support/Makefile   |   1 +
>  support/xdup.c     |  30 +++++++
>  support/xunistd.h  |   1 +
>  4 files changed, 95 insertions(+), 156 deletions(-)
>  create mode 100644 support/xdup.c
> 
> diff --git a/io/tst-faccessat.c b/io/tst-faccessat.c
> index b90954e318..609cdddaf7 100644
> --- a/io/tst-faccessat.c
> +++ b/io/tst-faccessat.c
> @@ -1,6 +1,23 @@
> -/* Test for faccessat function.  */
> -
> -#include <dirent.h>
> +/* Test for faccessat function.
> +   Copyright (C) 2006-2024 Free Software Foundation, Inc.
> +   Copyright The GNU Toolchain Authors.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
>  #include <fcntl.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> @@ -8,47 +25,19 @@
>  #include <unistd.h>
>  #include <sys/stat.h>
>  
> +#include <support/check.h>
> +#include <support/temp_file.h>
> +#include <support/test-driver.h>
> +#include <support/xdirent.h>
>  #include <support/xunistd.h>
>  
> -static void prepare (void);
> -#define PREPARE(argc, argv) prepare ()
> -
> -static int do_test (void);
> -#define TEST_FUNCTION do_test ()
> -
> -#include "../test-skeleton.c"
> -
>  static int dir_fd;
>  
>  static void
> -prepare (void)
> +prepare (int argc, char **argv)
>  {
> -  size_t test_dir_len = strlen (test_dir);
> -  static const char dir_name[] = "/tst-faccessat.XXXXXX";
> -
> -  size_t dirbuflen = test_dir_len + sizeof (dir_name);
> -  char *dirbuf = malloc (dirbuflen);
> -  if (dirbuf == NULL)
> -    {
> -      puts ("out of memory");
> -      exit (1);
> -    }
> -
> -  snprintf (dirbuf, dirbuflen, "%s%s", test_dir, dir_name);
> -  if (mkdtemp (dirbuf) == NULL)
> -    {
> -      puts ("cannot create temporary directory");
> -      exit (1);
> -    }
> -
> -  add_temp_file (dirbuf);
> -
> -  dir_fd = open (dirbuf, O_RDONLY | O_DIRECTORY);
> -  if (dir_fd == -1)
> -    {
> -      puts ("cannot open directory");
> -      exit (1);
> -    }
> +  dir_fd = xopen (support_create_temp_directory ("tst-faccessat."),
> +		  O_RDONLY | O_DIRECTORY, 0);
>  }
>  
>  
> @@ -56,159 +45,77 @@ static int
>  do_test (void)
>  {
>    /* fdopendir takes over the descriptor, make a copy.  */
> -  int dupfd = dup (dir_fd);
> -  if (dupfd == -1)
> -    {
> -      puts ("dup failed");
> -      return 1;
> -    }
> -  if (lseek (dupfd, 0, SEEK_SET) != 0)
> -    {
> -      puts ("1st lseek failed");
> -      return 1;
> -    }
> +  int dupfd = xdup (dir_fd);
> +  xlseek (dupfd, 0, SEEK_SET);
>  
>    /* The directory should be empty save the . and .. files.  */
> -  DIR *dir = fdopendir (dupfd);
> -  if (dir == NULL)
> -    {
> -      puts ("fdopendir failed");
> -      return 1;
> -    }
> +  DIR *dir = xfdopendir (dupfd);
> +
>    struct dirent64 *d;
> -  while ((d = readdir64 (dir)) != NULL)
> +  while ((d = xreaddir64 (dir)) != NULL)
>      if (strcmp (d->d_name, ".") != 0 && strcmp (d->d_name, "..") != 0)
> -      {
> -	printf ("temp directory contains file \"%s\"\n", d->d_name);
> -	return 1;
> -      }
> -  closedir (dir);
> +      FAIL_EXIT1 ("temp directory contains file \"%s\"\n", d->d_name);
> +  xclosedir (dir);
>  
>    /* Try to create a file.  */
>    int fd = openat (dir_fd, "some-file", O_CREAT|O_RDWR|O_EXCL, 0666);
>    if (fd == -1)
>      {
>        if (errno == ENOSYS)
> -	{
> -	  puts ("*at functions not supported");
> -	  return 0;
> -	}
> +	FAIL_UNSUPPORTED ("*at functions not supported");
>  
> -      puts ("file creation failed");
> -      return 1;
> +      FAIL_EXIT1 ("file creation failed");
>      }
>    xwrite (fd, "hello", 5);
>    puts ("file created");
>  
>    /* Before closing the file, try using this file descriptor to open
>       another file.  This must fail.  */
> -  if (faccessat (fd, "should-not-work", F_OK, AT_EACCESS) != -1)
> -    {
> -      puts ("faccessat using descriptor for normal file worked");
> -      return 1;
> -    }
> -  if (errno != ENOTDIR)
> -    {
> -      puts ("\
> -error for faccessat using descriptor for normal file not ENOTDIR ");
> -      return 1;
> -    }
> -
> -  close (fd);
> +  TEST_VERIFY_EXIT (faccessat (fd, "should-not-work", F_OK, AT_EACCESS) == -1);
> +  TEST_VERIFY_EXIT (errno == ENOTDIR);
>  
> -  int result = 0;
> +  xclose (fd);
>  
> -  if (faccessat (dir_fd, "some-file", F_OK, AT_EACCESS))
> -    {
> -      printf ("faccessat F_OK: %m\n");
> -      result = 1;
> -    }
> -  if (faccessat (dir_fd, "some-file", W_OK, AT_EACCESS))
> -    {
> -      printf ("faccessat W_OK: %m\n");
> -      result = 1;
> -    }
> +  TEST_VERIFY (faccessat (dir_fd, "some-file", F_OK, AT_EACCESS) == 0);
> +  TEST_VERIFY (faccessat (dir_fd, "some-file", W_OK, AT_EACCESS) == 0);
>  
>    errno = 0;
> -  if (faccessat (dir_fd, "some-file", X_OK, AT_EACCESS) == 0
> -      || errno != EACCES)
> -    {
> -      printf ("faccessat X_OK on nonexecutable: %m\n");
> -      result = 1;
> -    }
> +  TEST_VERIFY (faccessat (dir_fd, "some-file", X_OK, AT_EACCESS) != 0
> +	       && errno == EACCES);

Maybe move the errno to a different TEST_VERIFY, so on a possibly failure
it would be easier to sort out what has failed.

>  
>    if (fchmodat (dir_fd, "some-file", 0400, 0) != 0)
> -    {
> -      printf ("fchownat failed: %m\n");
> -      return 1;
> -    }
> +    FAIL_EXIT1 ("fchownat failed: %m\n");
>  
> -  if (faccessat (dir_fd, "some-file", R_OK, AT_EACCESS))
> -    {
> -      printf ("faccessat R_OK: %m\n");
> -      result = 1;
> -    }
> +  TEST_VERIFY (faccessat (dir_fd, "some-file", R_OK, AT_EACCESS) == 0);
>  
> +  /* Write would succeed only for EUID root, otherwise this test should
> +     fail.  */
>    errno = 0;
> -  if (faccessat (dir_fd, "some-file", W_OK, AT_EACCESS) == 0
> -      ? (geteuid () != 0) : (errno != EACCES))
> -    {
> -      printf ("faccessat W_OK on unwritable file: %m\n");
> -      result = 1;
> -    }
> +  TEST_VERIFY (faccessat (dir_fd, "some-file", W_OK, AT_EACCESS) == 0
> +	       ? (geteuid () == 0) : (errno == EACCES));
>  
>    /* Create a file descriptor which is closed again right away.  */
> -  int dir_fd2 = dup (dir_fd);
> -  if (dir_fd2 == -1)
> -    {
> -      puts ("dup failed");
> -      return 1;
> -    }
> +  int dir_fd2 = xdup (dir_fd);
>    close (dir_fd2);
>  
>    /* With the file descriptor closed the next call must fail.  */
> -  if (faccessat (dir_fd2, "some-file", F_OK, AT_EACCESS) != -1)
> -    {
> -      puts ("faccessat using closed descriptor succeeded");
> -      return 1;
> -    }
> -  if (errno != EBADF)
> -    {
> -      puts ("faccessat using closed descriptor did not set EBADF");
> -      return 1;
> -    }
> +  TEST_VERIFY_EXIT (faccessat (dir_fd2, "some-file", F_OK, AT_EACCESS)
> +		    == -1);
> +  TEST_VERIFY_EXIT (errno == EBADF);
>  
>    /* Same with a non-existing file.  */
> -  if (faccessat (dir_fd2, "non-existing-file", F_OK, AT_EACCESS) != -1)
> -    {
> -      puts ("2nd faccessat using closed descriptor succeeded");
> -      return 1;
> -    }
> -  if (errno != EBADF)
> -    {
> -      puts ("2nd faccessat using closed descriptor did not set EBADF");
> -      return 1;
> -    }
> +  TEST_VERIFY_EXIT (faccessat (dir_fd2, "non-existing-file", F_OK, AT_EACCESS)
> +		    == -1);
> +  TEST_VERIFY_EXIT (errno == EBADF);
>  
> -  if (unlinkat (dir_fd, "some-file", 0) != 0)
> -    {
> -      puts ("unlinkat failed");
> -      result = 1;
> -    }
> +  TEST_VERIFY (unlinkat (dir_fd, "some-file", 0) == 0);
>  
> -  close (dir_fd);
> +  xclose (dir_fd);
>  
> -  fd = faccessat (-1, "some-file", F_OK, AT_EACCESS);
> -  if (fd != -1)
> -    {
> -      puts ("faccessat using -1 descriptor succeeded");
> -      return 1;
> -    }
> -  if (errno != EBADF)
> -    {
> -      puts ("faccessat using -1 descriptor did not set EBADF");
> -      return 1;
> -    }
> +  TEST_VERIFY_EXIT (faccessat (-1, "some-file", F_OK, AT_EACCESS) == -1);
> +  TEST_VERIFY_EXIT (errno == EBADF);
>  
> -  return result;
> +  return 0;
>  }
> +#define PREPARE prepare
> +#include <support/test-driver.c>
> diff --git a/support/Makefile b/support/Makefile
> index 84e2419775..099f5ebb9c 100644
> --- a/support/Makefile
> +++ b/support/Makefile
> @@ -128,6 +128,7 @@ libsupport-routines = \
>    xcopy_file_range \
>    xdlfcn \
>    xdlmopen \
> +  xdup \
>    xdup2 \
>    xfchmod \
>    xfclose \
> diff --git a/support/xdup.c b/support/xdup.c
> new file mode 100644
> index 0000000000..1eab317354
> --- /dev/null
> +++ b/support/xdup.c
> @@ -0,0 +1,30 @@
> +/* dup with error checking.
> +   Copyright The GNU Toolchain Authors.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <support/xunistd.h>
> +#include <support/check.h>
> +
> +int
> +xdup (int from)
> +{
> +  int ret = dup (from);
> +  if (ret < 0)
> +    FAIL_EXIT1 ("dup (%d): %m", from);
> +
> +  return ret;
> +}
> diff --git a/support/xunistd.h b/support/xunistd.h
> index 204951bce7..0c6d837ac0 100644
> --- a/support/xunistd.h
> +++ b/support/xunistd.h
> @@ -35,6 +35,7 @@ pid_t xfork (void);
>  pid_t xwaitpid (pid_t, int *status, int flags);
>  void xpipe (int[2]);
>  void xdup2 (int, int);
> +int xdup (int);
>  int xopen (const char *path, int flags, mode_t);
>  void support_check_stat_fd (const char *name, int fd, int result);
>  void support_check_stat_path (const char *name, const char *path, int result);

I think the usual way is to add libsupport additions in a different
patch, so backports would be easier.


More information about the Libc-alpha mailing list