mbox series

[v4,00/15] Fix warn unused result

Message ID 20230428122142.928135-1-fberat@redhat.com
Headers show
Series Fix warn unused result | expand

Message

Frederic Berat April 28, 2023, 12:21 p.m. UTC
Hello,

Enabling _FORTIFY_SOURCE on Glibc itself implies that some function get the
__wur macro enabed.
This leads to build failures when -Werror is set.

I went for multiple patches for these fixes, a big one that cover the tests,
and then separate one for the other ones, assuming they may need to get a
closer look at.

While most of the changes look trivial, decision I made on how to fix them may
be open to debate, e.g. the additional checks may lead to failures while they
weren't earlier, some other have no impact while it may be useful to have one.

Feedback is therefore highly appreciated.

Fred.

Changes since v1:
 - "tests: fix warn unused result" has been split into several sub-patches. New
   x-variant of functions have been created when necessary.
 - "inet/rcmd.c" is removed has it has been merged upstream.
 - catgets/gencat.c:
   - introduced a static write_all routine, that mimics support/xwrite
   - fixed review findings (declaration of array_size where defined)
 - locale/programs/locarchive.c: untouched (I forgot it)
 - malloc/{memusage.c,memusagestat.c}: introduced write_all/read_all functions
 - nptl_db/thread_dbP.h: fixed missing white space.
 - sunrpc/netname.c: fixed missing whitespace, and "return 0;" indent.
 - sysdeps/pthread/eintr.c: Added comment on why the if statement is used.

Changes since v2:
 - Rebased on master
 - catgets/gencat.c: Don't use errno if write returns 0.
 - locale/programs/locarchive.c: Freeing normalized_codeset before returning.
 - malloc/{memusage.c,memusagestat.c}: Don't use errno if write returns 0.
 - tests: replace read by xread:
   - Add missing xunistd.h includes
   - Add libsupport to tst-fini1mod.so
 - tests: replace fread by xfread:
   - Fixed xfread prototype
   - Add missing xstdio.h include

Changes since v3:
 - Rebased on master

---
Frédéric Bérat (15):
  catgets/gencat.c: fix warn unused result
  locale/programs/locarchive.c: fix warn unused result
  malloc/{memusage.c,memusagestat.c}: fix warn unused result
  nptl_db/thread_dbP.h: fix warn unused result
  sunrpc/netname.c: fix warn unused result
  sysdeps/pthread/eintr.c: fix warn unused result
  tests: fix warn unused result on asprintf calls
  tests: replace write by xwrite
  tests: replace read by xread
  tests: replace system by xsystem
  tests: replace fread by xfread
  tests: replace ftruncate by xftruncate
  tests: replace fgets by xfgets
  tests: Replace various function calls with their x variant
  tests: fix warn unused results

 argp/argp-test.c               |  8 +++--
 assert/test-assert-perr.c      |  8 +++--
 assert/test-assert.c           |  8 +++--
 catgets/gencat.c               | 41 +++++++++++++++++++-------
 crypt/cert.c                   |  6 +++-
 dirent/tst-fdopendir.c         |  7 +++--
 elf/tst-stackguard1.c          |  2 +-
 io/tst-copy_file_range.c       |  2 +-
 io/tst-faccessat.c             |  3 +-
 io/tst-fchmodat.c              |  3 +-
 io/tst-fchownat.c              |  3 +-
 io/tst-fstatat.c               |  3 +-
 io/tst-futimesat.c             |  3 +-
 io/tst-linkat.c                |  3 +-
 io/tst-openat.c                |  3 +-
 io/tst-renameat.c              |  3 +-
 io/tst-symlinkat.c             |  3 +-
 io/tst-unlinkat.c              |  3 +-
 libio/bug-fseek.c              |  7 +++--
 libio/bug-mmap-fflush.c        |  7 +++--
 libio/bug-ungetc.c             |  4 ++-
 libio/bug-ungetc3.c            |  4 ++-
 libio/bug-ungetc4.c            |  4 ++-
 libio/bug-wfflush.c            |  4 ++-
 libio/bug-wsetpos.c            |  4 ++-
 locale/programs/locarchive.c   | 24 ++++++++++-----
 malloc/memusage.c              | 53 ++++++++++++++++++++++++++--------
 malloc/memusagestat.c          | 48 +++++++++++++++++++++++++++---
 misc/tst-efgcvt-template.c     |  4 +--
 misc/tst-error1.c              |  2 +-
 nptl/tst-cancel7.c             |  3 +-
 nptl/tst-cleanup4.c            |  4 ++-
 nptl/tst-stackguard1.c         |  6 ++--
 nptl/tst-tls3.c                |  2 ++
 nptl/tst-tls3mod.c             |  5 ++--
 nptl_db/thread_dbP.h           |  2 +-
 nss/tst-nss-db-endpwent.c      |  3 +-
 nss/tst-reload2.c              |  6 +++-
 posix/tst-chmod.c              |  9 ++++--
 posix/tst-execl2.c             |  8 ++---
 posix/tst-execle2.c            |  8 ++---
 posix/tst-execlp2.c            | 11 ++-----
 posix/tst-execv2.c             |  8 ++---
 posix/tst-execve2.c            |  8 ++---
 posix/tst-execvp2.c            | 17 +++--------
 posix/tst-getopt-cancel.c      |  3 +-
 posix/tst-nice.c               |  3 +-
 posix/wordexp-test.c           | 12 ++++++--
 rt/tst-cpuclock2.c             |  4 ++-
 rt/tst-cputimer1.c             |  4 ++-
 rt/tst-cputimer2.c             |  4 ++-
 rt/tst-cputimer3.c             |  4 ++-
 stdio-common/bug12.c           | 12 ++++----
 stdio-common/bug19.c           |  9 ++++--
 stdio-common/bug3.c            |  4 ++-
 stdio-common/bug4.c            |  4 ++-
 stdio-common/bug5.c            |  4 ++-
 stdio-common/bug6.c            |  8 ++---
 stdio-common/test-fwrite.c     |  4 ++-
 stdio-common/test_rdwr.c       | 11 +++----
 stdio-common/tst-cookie.c      |  5 +++-
 stdio-common/tst-fmemopen3.c   |  4 ++-
 stdio-common/tst-fseek.c       |  5 ++--
 stdio-common/tst-perror.c      |  3 +-
 stdio-common/tstscanf.c        | 14 +++++++--
 stdlib/test-canon.c            | 25 +++++++++++++---
 sunrpc/netname.c               |  3 +-
 support/Makefile               |  4 +++
 support/test-container.c       | 15 +++++-----
 support/xfgets.c               | 32 ++++++++++++++++++++
 support/xfread.c               | 39 +++++++++++++++++++++++++
 support/xread.c                | 36 +++++++++++++++++++++++
 support/xstdio.h               |  2 ++
 support/xstdlib.h              | 31 ++++++++++++++++++++
 support/xsystem.c              | 29 +++++++++++++++++++
 support/xunistd.h              |  3 ++
 sysdeps/pthread/Makefile       |  2 +-
 sysdeps/pthread/eintr.c        |  6 ++--
 sysdeps/pthread/tst-cancel11.c |  4 ++-
 sysdeps/pthread/tst-cancel16.c |  6 +++-
 sysdeps/pthread/tst-cancel20.c | 10 ++-----
 sysdeps/pthread/tst-cancel21.c |  9 ++----
 sysdeps/pthread/tst-cancel4.c  |  6 ++--
 sysdeps/pthread/tst-cancel6.c  |  3 +-
 sysdeps/pthread/tst-cond18.c   |  4 ++-
 sysdeps/pthread/tst-fini1mod.c |  4 ++-
 sysdeps/pthread/tst-flock1.c   |  3 +-
 sysdeps/pthread/tst-flock2.c   |  3 +-
 sysdeps/pthread/tst-key1.c     | 11 +++----
 sysdeps/pthread/tst-signal1.c  |  3 +-
 sysdeps/pthread/tst-signal2.c  |  3 +-
 sysdeps/pthread/tst-timer.c    |  3 +-
 time/tst-cpuclock1.c           |  4 ++-
 93 files changed, 587 insertions(+), 214 deletions(-)
 create mode 100644 support/xfgets.c
 create mode 100644 support/xfread.c
 create mode 100644 support/xread.c
 create mode 100644 support/xstdlib.h
 create mode 100644 support/xsystem.c

Comments

Siddhesh Poyarekar May 25, 2023, 1:53 a.m. UTC | #1
On 2023-04-28 08:21, Frédéric Bérat wrote:
> Hello,
> 
> Enabling _FORTIFY_SOURCE on Glibc itself implies that some function get the
> __wur macro enabed.
> This leads to build failures when -Werror is set.
> 
> I went for multiple patches for these fixes, a big one that cover the tests,
> and then separate one for the other ones, assuming they may need to get a
> closer look at.
> 
> While most of the changes look trivial, decision I made on how to fix them may
> be open to debate, e.g. the additional checks may lead to failures while they
> weren't earlier, some other have no impact while it may be useful to have one.
> 
> Feedback is therefore highly appreciated.

Thank you for your continued work on this; 3 down, 12 (and a bit more 
;)) to go.

I have pushed 3 of the patches I get R-b for.  Overall for the remaining 
patches, there's one review note that's pertinent; the tests that you've 
added the x* function calls for need to use the support.h system, i.e. 
replace test-skeleton.c with support/test-driver.c and then replace the 
non-zero returns from do_test with the check.h macros.

Thanks,
Sid