Re: [PATCH] getdelim: ensure error indicator is set on error (bug 29917)

[Carlos O'Donell <carlos@redhat.com>]

On 12/21/22 04:29, Andreas Schwab wrote:
> POSIX requires that getdelim and getline set the error indicator on the
> stream when an error occured, in addition to setting errno.

LGTM. Thank you.

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

> ---
>  libio/Makefile       |  2 +-
>  libio/iogetdelim.c   | 16 +++++++++++-----
>  libio/tst-getdelim.c | 36 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 48 insertions(+), 6 deletions(-)
>  create mode 100644 libio/tst-getdelim.c
> 
> diff --git a/libio/Makefile b/libio/Makefile
> index 64398ab1ee..9c69a85c87 100644
> --- a/libio/Makefile
> +++ b/libio/Makefile
> @@ -66,7 +66,7 @@ tests = tst_swprintf tst_wprintf tst_swscanf tst_wscanf tst_getwc tst_putwc   \
>  	tst-fwrite-error tst-ftell-partial-wide tst-ftell-active-handler \
>  	tst-ftell-append tst-fputws tst-bz22415 tst-fgetc-after-eof \
>  	tst-sprintf-ub tst-sprintf-chk-ub tst-bz24051 tst-bz24153 \
> -	tst-wfile-sync tst-bz28828
> +	tst-wfile-sync tst-bz28828 tst-getdelim
>  
>  tests-internal = tst-vtables tst-vtables-interposed
>  
> diff --git a/libio/iogetdelim.c b/libio/iogetdelim.c
> index b6c4c07b45..591526e9c1 100644
> --- a/libio/iogetdelim.c
> +++ b/libio/iogetdelim.c
> @@ -43,11 +43,6 @@ __getdelim (char **lineptr, size_t *n, int delimiter, FILE *fp)
>    ssize_t cur_len = 0;
>    ssize_t len;
>  
> -  if (lineptr == NULL || n == NULL)
> -    {
> -      __set_errno (EINVAL);
> -      return -1;
> -    }
>    CHECK_FILE (fp, -1);
>    _IO_acquire_lock (fp);
>    if (_IO_ferror_unlocked (fp))
> @@ -56,12 +51,21 @@ __getdelim (char **lineptr, size_t *n, int delimiter, FILE *fp)
>        goto unlock_return;
>      }
>  
> +  if (lineptr == NULL || n == NULL)
> +    {
> +      __set_errno (EINVAL);
> +      fseterr_unlocked (fp);
> +      result = -1;
> +      goto unlock_return;
> +    }
> +
>    if (*lineptr == NULL || *n == 0)
>      {
>        *n = 120;
>        *lineptr = (char *) malloc (*n);
>        if (*lineptr == NULL)
>  	{
> +	  fseterr_unlocked (fp);
>  	  result = -1;
>  	  goto unlock_return;
>  	}
> @@ -88,6 +92,7 @@ __getdelim (char **lineptr, size_t *n, int delimiter, FILE *fp)
>        if (__glibc_unlikely (len >= SSIZE_MAX - cur_len))
>  	{
>  	  __set_errno (EOVERFLOW);
> +	  fseterr_unlocked (fp);
>  	  result = -1;
>  	  goto unlock_return;
>  	}
> @@ -102,6 +107,7 @@ __getdelim (char **lineptr, size_t *n, int delimiter, FILE *fp)
>  	  new_lineptr = (char *) realloc (*lineptr, needed);
>  	  if (new_lineptr == NULL)
>  	    {
> +	      fseterr_unlocked (fp);
>  	      result = -1;
>  	      goto unlock_return;
>  	    }
> diff --git a/libio/tst-getdelim.c b/libio/tst-getdelim.c
> new file mode 100644
> index 0000000000..1def193e8d
> --- /dev/null
> +++ b/libio/tst-getdelim.c
> @@ -0,0 +1,36 @@
> +/* Check that getdelim sets error indicator on error (BZ #29917)
> +
> +   Copyright (C) 2022 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 <stdio.h>
> +#include <errno.h>
> +
> +#include <support/check.h>
> +
> +static int
> +do_test (void)
> +{
> +  clearerr (stdin);
> +  TEST_VERIFY (getdelim (0, 0, '\n', stdin) == -1);

OK. s/EOF/-1/g is corrected. Thanks.

> +  TEST_VERIFY (ferror (stdin) != 0);
> +  TEST_VERIFY (errno == EINVAL);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

-- 
Cheers,
Carlos.