From 20b6b8e8a50874dd189687df8f6f8a11d4813b34 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Fr=C3=A9d=C3=A9ric=20B=C3=A9rat?= Date: Wed, 14 Jun 2023 10:52:06 +0200 Subject: [PATCH] tests: replace read by xread With fortification enabled, read calls return result needs to be checked, has it gets the __wur macro enabled. Note on read call removal from sysdeps/pthread/tst-cancel20.c and sysdeps/pthread/tst-cancel21.c: It is assumed that this second read call was there to overcome the race condition between pipe closure and thread cancellation that could happen in the original code. Since this race condition got fixed by d0e3ffb7a58854248f1d5e737610d50cd0a60f46 the second call seems superfluous. Hence, instead of checking for the return value of read, it looks reasonable to simply remove it. Reviewed-by: Siddhesh Poyarekar --- dirent/tst-fdopendir.c | 3 ++- nptl/tst-cleanup4.c | 4 +++- support/Makefile | 1 + support/test-container.c | 3 ++- support/xread.c | 36 ++++++++++++++++++++++++++++++++++ support/xunistd.h | 3 +++ sysdeps/pthread/Makefile | 2 +- sysdeps/pthread/tst-cancel11.c | 6 +++++- sysdeps/pthread/tst-cancel20.c | 2 -- sysdeps/pthread/tst-cancel21.c | 2 -- sysdeps/pthread/tst-fini1mod.c | 4 +++- 11 files changed, 56 insertions(+), 10 deletions(-) create mode 100644 support/xread.c diff --git a/dirent/tst-fdopendir.c b/dirent/tst-fdopendir.c index 2c9520574dd..d6a24f47db2 100644 --- a/dirent/tst-fdopendir.c +++ b/dirent/tst-fdopendir.c @@ -45,7 +45,8 @@ do_test (void) } char buf[5]; - read(fd, buf, sizeof (buf)); + xread(fd, buf, sizeof (buf)); + close(fd); struct stat64 st2; diff --git a/nptl/tst-cleanup4.c b/nptl/tst-cleanup4.c index 1d3d53fb5f1..f2e9f263e5c 100644 --- a/nptl/tst-cleanup4.c +++ b/nptl/tst-cleanup4.c @@ -21,6 +21,8 @@ #include #include +#include + /* LinuxThreads pthread_cleanup_{push,pop} helpers. */ extern void _pthread_cleanup_push (struct _pthread_cleanup_buffer *__buffer, void (*__routine) (void *), @@ -64,7 +66,7 @@ fn_read (void) } char c; - read (fds[0], &c, 1); + xread (fds[0], &c, 1); } diff --git a/support/Makefile b/support/Makefile index 8ebe68aa95c..5939e534698 100644 --- a/support/Makefile +++ b/support/Makefile @@ -197,6 +197,7 @@ libsupport-routines = \ xpthread_spin_lock \ xpthread_spin_unlock \ xraise \ + xread \ xreadlink \ xrealloc \ xrecvfrom \ diff --git a/support/test-container.c b/support/test-container.c index 20ea19af37a..788b091ea08 100644 --- a/support/test-container.c +++ b/support/test-container.c @@ -1217,7 +1217,8 @@ main (int argc, char **argv) /* Get our "outside" pid from our parent. We use this to help with debugging from outside the container. */ - read (pipes[0], &child, sizeof(child)); + xread (pipes[0], &child, sizeof(child)); + close (pipes[0]); close (pipes[1]); sprintf (pid_buf, "%lu", (long unsigned)child); diff --git a/support/xread.c b/support/xread.c new file mode 100644 index 00000000000..19f76fcfeb7 --- /dev/null +++ b/support/xread.c @@ -0,0 +1,36 @@ +/* read with error checking and retries. + Copyright (C) 2023 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 + . */ + +#include + +#include + +void +xread (int fd, void *buffer, size_t length) +{ + char *p = buffer; + char *end = p + length; + while (p < end) + { + ssize_t ret = read (fd, p, end - p); + if (ret < 0) + FAIL_EXIT1 ("read of %zu bytes failed after %td: %m", + length, p - (char *) buffer); + p += ret; + } +} diff --git a/support/xunistd.h b/support/xunistd.h index 43a1e69fcb9..0aa2638a8da 100644 --- a/support/xunistd.h +++ b/support/xunistd.h @@ -77,6 +77,9 @@ void xclose (int); /* Write the buffer. Retry on short writes. */ void xwrite (int, const void *, size_t); +/* Read to buffer. Retry on short reads. */ +void xread (int, void *, size_t); + /* Invoke mmap with a zero file offset. */ void *xmmap (void *addr, size_t length, int prot, int flags, int fd); void xmprotect (void *addr, size_t length, int prot); diff --git a/sysdeps/pthread/Makefile b/sysdeps/pthread/Makefile index 5df1109dd3a..32cf4eb119c 100644 --- a/sysdeps/pthread/Makefile +++ b/sysdeps/pthread/Makefile @@ -464,7 +464,7 @@ $(objpfx)tst-cancel28: $(librt) $(objpfx)tst-fini1: $(shared-thread-library) $(objpfx)tst-fini1mod.so -$(objpfx)tst-fini1mod.so: $(shared-thread-library) +$(objpfx)tst-fini1mod.so: $(libsupport) $(shared-thread-library) $(objpfx)tst-_res1mod2.so: $(objpfx)tst-_res1mod1.so LDFLAGS-tst-_res1mod1.so = -Wl,-soname,tst-_res1mod1.so diff --git a/sysdeps/pthread/tst-cancel11.c b/sysdeps/pthread/tst-cancel11.c index 4dd84d6673c..0fdc4441542 100644 --- a/sysdeps/pthread/tst-cancel11.c +++ b/sysdeps/pthread/tst-cancel11.c @@ -56,7 +56,11 @@ tf (void *arg) /* This call should block and be cancelable. */ char buf[20]; - read (fd[0], buf, sizeof (buf)); + if (read (fd[0], buf, sizeof (buf))) + { + puts ("read unexpectedly returned"); + exit (1); + } pthread_cleanup_pop (0); diff --git a/sysdeps/pthread/tst-cancel20.c b/sysdeps/pthread/tst-cancel20.c index 1d5c53049bf..93f287aa4f4 100644 --- a/sysdeps/pthread/tst-cancel20.c +++ b/sysdeps/pthread/tst-cancel20.c @@ -84,8 +84,6 @@ tf_body (void) exit (1); } - read (fd[0], &c, 1); - pthread_cleanup_pop (0); } diff --git a/sysdeps/pthread/tst-cancel21.c b/sysdeps/pthread/tst-cancel21.c index bc4ff308f93..ec8bf0dba30 100644 --- a/sysdeps/pthread/tst-cancel21.c +++ b/sysdeps/pthread/tst-cancel21.c @@ -85,8 +85,6 @@ tf_body (void) exit (1); } - read (fd[0], &c, 1); - pthread_cleanup_pop (0); } diff --git a/sysdeps/pthread/tst-fini1mod.c b/sysdeps/pthread/tst-fini1mod.c index cdadf034cd7..0a45f6c5f2f 100644 --- a/sysdeps/pthread/tst-fini1mod.c +++ b/sysdeps/pthread/tst-fini1mod.c @@ -20,6 +20,8 @@ #include #include +#include + static void * tf (void *arg) @@ -32,7 +34,7 @@ tf (void *arg) } char buf[10]; - read (fds[0], buf, sizeof (buf)); + xread (fds[0], buf, sizeof (buf)); puts ("read returned"); exit (1); -- 2.47.2