[08/11] gshadow: Implement fgetsgent_r using __nss_fgetent_r (bug 20338)

  ---
 gshadow/Makefile          |   2 +-
 gshadow/fgetsgent_r.c     |  41 ++------
 gshadow/tst-fgetsgent_r.c | 191 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 198 insertions(+), 36 deletions(-)
 create mode 100644 gshadow/tst-fgetsgent_r.c
  

On 7/17/20 4:30 AM, Florian Weimer via Libc-alpha wrote:

Huh, there isn't a man page for fgetsgent_r :}

OK for 2.32. Good cleanup and test. Implement with __nss_fgetent_r and
because we can use a FILE* we can test it with an arbitrary file.

Tested-by: Carlos O'Donell <carlos@redhat.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>

> ---
>  gshadow/Makefile          |   2 +-
>  gshadow/fgetsgent_r.c     |  41 ++------
>  gshadow/tst-fgetsgent_r.c | 191 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 198 insertions(+), 36 deletions(-)
>  create mode 100644 gshadow/tst-fgetsgent_r.c
> 
> diff --git a/gshadow/Makefile b/gshadow/Makefile
> index 089aa6125f..32faf7a534 100644
> --- a/gshadow/Makefile
> +++ b/gshadow/Makefile
> @@ -26,7 +26,7 @@ headers		= gshadow.h
>  routines	= getsgent getsgnam sgetsgent fgetsgent putsgent \
>  		  getsgent_r getsgnam_r sgetsgent_r fgetsgent_r
>  
> -tests = tst-gshadow tst-putsgent
> +tests = tst-gshadow tst-putsgent tst-fgetsgent_r
>  
>  CFLAGS-getsgent_r.c += -fexceptions
>  CFLAGS-getsgent.c += -fexceptions
> diff --git a/gshadow/fgetsgent_r.c b/gshadow/fgetsgent_r.c
> index a7a1860c76..218206b4ac 100644
> --- a/gshadow/fgetsgent_r.c
> +++ b/gshadow/fgetsgent_r.c
> @@ -36,40 +36,11 @@ int
>  __fgetsgent_r (FILE *stream, struct sgrp *resbuf, char *buffer, size_t buflen,
>  	       struct sgrp **result)
>  {
> -  char *p;
> -
> -  _IO_flockfile (stream);
> -  do
> -    {
> -      buffer[buflen - 1] = '\xff';
> -      p = fgets_unlocked (buffer, buflen, stream);
> -      if (p == NULL && feof_unlocked (stream))
> -	{
> -	  _IO_funlockfile (stream);
> -	  *result = NULL;
> -	  __set_errno (ENOENT);
> -	  return errno;
> -	}
> -      if (p == NULL || buffer[buflen - 1] != '\xff')
> -	{
> -	  _IO_funlockfile (stream);
> -	  *result = NULL;
> -	  __set_errno (ERANGE);
> -	  return errno;
> -	}
> -
> -      /* Skip leading blanks.  */
> -      while (isspace (*p))
> -	++p;
> -    } while (*p == '\0' || *p == '#' /* Ignore empty and comment lines.  */
> -	     /* Parse the line.  If it is invalid, loop to
> -		get the next line of the file to parse.  */
> -	     || ! parse_line (buffer, (void *) resbuf, (void *) buffer, buflen,
> -			      &errno));
> -
> -  _IO_funlockfile (stream);
> -
> -  *result = resbuf;
> -  return 0;
> +  int ret = __nss_fgetent_r (stream, resbuf, buffer, buflen, parse_line);
> +  if (ret == 0)
> +    *result = resbuf;
> +  else
> +    *result = NULL;
> +  return ret;
>  }
>  weak_alias (__fgetsgent_r, fgetsgent_r)
> diff --git a/gshadow/tst-fgetsgent_r.c b/gshadow/tst-fgetsgent_r.c
> new file mode 100644
> index 0000000000..780409afa9
> --- /dev/null
> +++ b/gshadow/tst-fgetsgent_r.c
> @@ -0,0 +1,191 @@
> +/* Test for fgetsgent_r and buffer sizes.
> +   Copyright (C) 2020 Free Software Foundation, Inc.
> +   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 <array_length.h>
> +#include <errno.h>
> +#include <gshadow.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/temp_file.h>
> +#include <support/xmemstream.h>
> +#include <support/xstdio.h>
> +
> +/* Turn a parsed struct back into a line string.  The returned string
> +   should be freed.  */
> +static char *
> +format_ent (const struct sgrp *e)
> +{
> +  struct xmemstream stream;
> +  xopen_memstream (&stream);
> +  TEST_COMPARE (putsgent (e, stream.out), 0);
> +  xfclose_memstream (&stream);
> +  return stream.buffer;
> +}
> +
> +/* An entry in the input file along with the expected output.  */
> +struct input
> +{
> +  const char *line;		/* Line in the file.  */
> +  const char *expected;		/* Expected output.  NULL if skipped.  */
> +};
> +
> +const struct input inputs[] =
> +  {
> +   /* Regular entries.  */
> +   { "g1:x1::\n", "g1:x1::\n" },
> +   { "g2:x2:a1:\n", "g2:x2:a1:\n" },
> +   { "g3:x3:a2:u1\n", "g3:x3:a2:u1\n" },
> +   { "g4:x4:a3,a4:u2,u3,u4\n", "g4:x4:a3,a4:u2,u3,u4\n" },
> +
> +   /* Comments and empty lines.  */
> +   { "\n", NULL },
> +   { " \n", NULL },
> +   { "\t\n", NULL },
> +   { "#g:x::\n", NULL },
> +   { " #g:x::\n", NULL },
> +   { "\t#g:x::\n", NULL },
> +   { " \t#g:x::\n", NULL },
> +
> +   /* Marker for synchronization.  */
> +   { "g5:x5::\n", "g5:x5::\n" },
> +
> +   /* Leading whitespace.  */
> +   { " g6:x6::\n", "g6:x6::\n" },
> +   { "\tg7:x7::\n", "g7:x7::\n" },
> +
> +   /* This is expected to trigger buffer exhaustion during parsing
> +      (bug 20338).  */
> +   {
> +    "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n",
> +    "g8:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx:u5,u6,u7,u8,u9:\n",
> +   },
> +   {
> +    "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n",
> +    "g9:xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx::a5,a6,a7,a8,a9,a10\n",
> +   },
> +  };
> +
> +/* Writes the test data to a temporary file and returns its name.  The
> +   returned pointer should be freed.  */
> +static char *
> +create_test_file (void)
> +{
> +  char *path;
> +  int fd = create_temp_file ("tst-fgetsgent_r-", &path);
> +  FILE *fp = fdopen (fd, "w");
> +  TEST_VERIFY_EXIT (fp != NULL);
> +
> +  for (size_t i = 0; i < array_length (inputs); ++i)
> +    fputs (inputs[i].line, fp);
> +
> +  xfclose (fp);
> +  return path;
> +}
> +
> +/* Read the test file with the indicated start buffer size.  Return
> +   true if the buffer size had to be increased during reading.  */
> +static bool
> +run_test (const char *path, size_t buffer_size)
> +{
> +  bool resized = false;
> +  FILE *fp = xfopen (path, "r");
> +
> +  /* This avoids repeated lseek system calls (bug 26257).  */
> +  TEST_COMPARE (fseeko64 (fp, 0, SEEK_SET), 0);
> +
> +  size_t i = 0;
> +  while (true)
> +    {
> +      /* Skip over unused expected entries.  */
> +      while (i < array_length (inputs) && inputs[i].expected == NULL)
> +	++i;
> +
> +      /* Store the data on the heap, to help valgrind to detect
> +	 invalid accesses.  */
> +      struct sgrp *result_storage = xmalloc (sizeof (*result_storage));
> +      char *buffer = xmalloc (buffer_size);
> +      struct sgrp **result_pointer_storage
> +	= xmalloc (sizeof (*result_pointer_storage));
> +
> +      int ret = fgetsgent_r (fp, result_storage, buffer, buffer_size,
> +			     result_pointer_storage);
> +      if (ret == 0)
> +	{
> +	  TEST_VERIFY (*result_pointer_storage != NULL);
> +	  TEST_VERIFY (i < array_length (inputs));
> +	  if (*result_pointer_storage != NULL
> +	      && i < array_length (inputs))
> +	    {
> +	      char * actual = format_ent (*result_pointer_storage);
> +	      TEST_COMPARE_STRING (inputs[i].expected, actual);
> +	      free (actual);
> +	      ++i;
> +	    }
> +	  else
> +	    break;
> +	}
> +      else
> +	{
> +	  TEST_VERIFY (*result_pointer_storage == NULL);
> +	  TEST_COMPARE (ret, errno);
> +
> +	  if (ret == ENOENT)
> +	    {
> +	      TEST_COMPARE (i, array_length (inputs));
> +	      free (result_pointer_storage);
> +	      free (buffer);
> +	      free (result_storage);
> +	      break;
> +	    }
> +	  else if (ret == ERANGE)
> +	    {
> +	      resized = true;
> +	      ++buffer_size;
> +	    }
> +	  else
> +	    FAIL_EXIT1 ("read failure: %m");
> +	}
> +
> +      free (result_pointer_storage);
> +      free (buffer);
> +      free (result_storage);
> +    }
> +
> +  return resized;
> +}
> +
> +static int
> +do_test (void)
> +{
> +  char *path = create_test_file ();
> +
> +  for (size_t buffer_size = 3; ; ++buffer_size)
> +    {
> +      bool resized = run_test (path, buffer_size);
> +      if (!resized)
> +	break;
> +    }
> +
> +  free (path);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>
>