getdelim: ensure error indicator is set on error (bug 29917)

  POSIX requires that getdelim and getline set the error indicator on the
stream when an error occured, in addition to setting errno.
---
 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
  

On 12/19/22 11:01, 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.

Minor test case fix requested.

Please post the final version and I'll review again.

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

OK. Add new test. Ugh, this tests list should be sorted. I'll push that after your change.

>  
>  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;
> -    }

OK. Don't do the checking early...

>    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;
> +    }

OK. Wait until after we lock the file and use the 'goto unlock_return' path.

> +
>    if (*lineptr == NULL || *n == 0)
>      {
>        *n = 120;
>        *lineptr = (char *) malloc (*n);
>        if (*lineptr == NULL)
>  	{
> +	  fseterr_unlocked (fp);

OK. 1/3 paths fixed.

>  	  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);

OK. 2/3 paths fixed.

>  	  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);

OK. 3/3 paths fixed.

>  	      result = -1;
>  	      goto unlock_return;
>  	    }
> diff --git a/libio/tst-getdelim.c b/libio/tst-getdelim.c
> new file mode 100644
> index 0000000000..a43674bf20
> --- /dev/null
> +++ b/libio/tst-getdelim.c
> @@ -0,0 +1,36 @@
> +/* Check that getdelim sets error indicator on error (BZ #29917)

OK.

> +
> +   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);

OK.

> +  TEST_VERIFY (getdelim (0, 0, '\n', stdin) == EOF);

We can't assume it returns EOF. It returns the number of bytes read
or -1, but not EOF (POSIX Issue 7).

> +  TEST_VERIFY (ferror (stdin) != 0);

OK.

> +  TEST_VERIFY (errno == EINVAL);

OK.

> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>