Re: [PATCH 1/8] tests: fix warn unused result

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2023-04-18 08:11, Frédéric Bérat via Libc-alpha wrote:
> When enabling _FORTIFY_SOURCE, some functions now lead to warnings when
> their result is not checked.

I haven't done a full review of this patch yet, but the first straight 
issue I can see is that you could use the x* functions in support/ in 
the tests to avoid repeating the error checking blocks in tests.  For 
example:

> ---
>   argp/argp-test.c               | 14 +++++++++++---
>   assert/test-assert-perr.c      | 13 +++++++------
>   assert/test-assert.c           | 13 +++++++------
>   crypt/cert.c                   |  6 +++++-
>   dirent/tst-fdopendir.c         | 13 +++++++++++--
>   elf/tst-stackguard1.c          |  2 +-
>   io/tst-copy_file_range.c       |  2 +-
>   io/tst-faccessat.c             |  6 +++++-
>   io/tst-fchmodat.c              |  6 +++++-
>   io/tst-fchownat.c              |  6 +++++-
>   io/tst-fstatat.c               |  6 +++++-
>   io/tst-futimesat.c             |  6 +++++-
>   io/tst-linkat.c                |  6 +++++-
>   io/tst-openat.c                |  6 +++++-
>   io/tst-renameat.c              |  6 +++++-
>   io/tst-symlinkat.c             |  6 +++++-
>   io/tst-unlinkat.c              |  6 +++++-
>   libio/bug-fseek.c              | 15 ++++++++++++---
>   libio/bug-mmap-fflush.c        | 12 ++++++++++--
>   libio/bug-ungetc.c             |  7 ++++++-
>   libio/bug-ungetc3.c            |  7 ++++++-
>   libio/bug-ungetc4.c            |  7 ++++++-
>   libio/bug-wfflush.c            |  6 +++++-
>   libio/bug-wsetpos.c            |  7 ++++++-
>   misc/tst-efgcvt-template.c     |  4 ++--
>   misc/tst-error1.c              |  2 +-
>   nptl/tst-cancel7.c             |  2 +-
>   nptl/tst-cleanup4.c            |  6 +++++-
>   nptl/tst-stackguard1.c         |  8 ++++++--
>   nptl/tst-tls3mod.c             |  4 ++--
>   nss/tst-nss-db-endpwent.c      |  6 +++++-
>   nss/tst-reload2.c              |  6 +++++-
>   posix/tst-chmod.c              |  9 +++++++--
>   posix/tst-execl2.c             |  4 ++--
>   posix/tst-execle2.c            |  4 ++--
>   posix/tst-execlp2.c            |  7 +++----
>   posix/tst-execv2.c             |  3 +--
>   posix/tst-execve2.c            |  3 +--
>   posix/tst-execvp2.c            |  6 ++----
>   posix/tst-getopt-cancel.c      |  2 +-
>   posix/tst-nice.c               |  3 +--
>   posix/wordexp-test.c           | 12 ++++++++++--
>   rt/tst-cpuclock2.c             |  2 +-
>   rt/tst-cputimer1.c             |  2 +-
>   rt/tst-cputimer2.c             |  2 +-
>   rt/tst-cputimer3.c             |  2 +-
>   stdio-common/bug12.c           | 15 ++++++++++-----
>   stdio-common/bug19.c           |  9 +++++++--
>   stdio-common/bug3.c            |  6 +++++-
>   stdio-common/bug4.c            |  6 +++++-
>   stdio-common/bug5.c            |  6 +++++-
>   stdio-common/bug6.c            |  8 ++++----
>   stdio-common/test-fwrite.c     |  8 ++++++--
>   stdio-common/test_rdwr.c       |  3 ++-
>   stdio-common/tst-cookie.c      |  4 +++-
>   stdio-common/tst-fmemopen3.c   |  6 +++++-
>   stdio-common/tst-fseek.c       |  3 +--
>   stdio-common/tst-perror.c      |  6 +++++-
>   stdio-common/tstscanf.c        | 14 ++++++++++++--
>   stdlib/test-canon.c            | 25 +++++++++++++++++++++----
>   support/test-container.c       | 18 +++++++++++-------
>   sysdeps/pthread/tst-cancel11.c |  6 +++++-
>   sysdeps/pthread/tst-cancel16.c |  6 +++++-
>   sysdeps/pthread/tst-cancel20.c |  2 --
>   sysdeps/pthread/tst-cancel21.c |  2 --
>   sysdeps/pthread/tst-cancel4.c  |  6 ++++--
>   sysdeps/pthread/tst-cancel6.c  |  3 ++-
>   sysdeps/pthread/tst-cond18.c   |  2 +-
>   sysdeps/pthread/tst-fini1mod.c |  6 +++++-
>   sysdeps/pthread/tst-flock1.c   |  6 +++++-
>   sysdeps/pthread/tst-flock2.c   |  6 +++++-
>   sysdeps/pthread/tst-key1.c     | 10 +++++-----
>   sysdeps/pthread/tst-signal1.c  |  6 +++++-
>   sysdeps/pthread/tst-signal2.c  |  6 +++++-
>   sysdeps/pthread/tst-timer.c    |  2 +-
>   time/tst-cpuclock1.c           |  2 +-
>   76 files changed, 356 insertions(+), 135 deletions(-)
> 
> diff --git a/argp/argp-test.c b/argp/argp-test.c
> index c7e20f6235..fa7f9694d1 100644
> --- a/argp/argp-test.c
> +++ b/argp/argp-test.c
> @@ -178,12 +178,20 @@ help_filter (int key, const char *text, void *input)
>     if (key == ARGP_KEY_HELP_POST_DOC && text)
>       {
>         time_t now = time (0);
> -      asprintf (&new_text, text, ctime (&now));
> +      if (asprintf (&new_text, text, ctime (&now)) < 0)

... this could be xasprintf,

> +	{
> +	  printf("Failed to execute asprintf: %m\n");
> +	  exit(1);
> +	}
>       }
>     else if (key == 'f')
>       /* Show the default for the --foonly option.  */
> -    asprintf (&new_text, "%s (ZOT defaults to %x)",
> -	      text, params->foonly_default);
> +    if (asprintf (&new_text, "%s (ZOT defaults to %x)",
> +	      text, params->foonly_default) < 0)
> +	{
> +	  printf("Failed to execute asprintf: %m\n");
> +	  exit(1);
> +	}
>     else
>       new_text = (char *)text;
>   
> diff --git a/assert/test-assert-perr.c b/assert/test-assert-perr.c
> index 8496db6ffd..ecfd4d14e5 100644
> --- a/assert/test-assert-perr.c
> +++ b/assert/test-assert-perr.c
> @@ -46,6 +46,7 @@ int
>   main(void)
>   {
>     volatile int failed = 1;  /* safety in presence of longjmp() */
> +  char *ret;
>   
>     fclose (stderr);
>     stderr = tmpfile ();
> @@ -70,16 +71,16 @@ main(void)
>       failed = 1; /* should not happen */
>   
>     rewind (stderr);
> -  fgets (buf, 160, stderr);
> -  if (!strstr(buf, strerror (1)))
> +  ret = fgets (buf, 160, stderr);
> +  if ((!ret && ferror(stderr)) || !strstr(buf, strerror (1)))

This could be a new xfgets, and so on.

I'd suggest making these test fixes into their own series, grouped by 
the x* functions, i.e. one patch for xasprintf, one for xfgets, etc.

Thanks,
Sid