public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "Frédéric Bérat" <fberat@redhat.com>
To: libc-alpha@sourceware.org
Cc: siddhesh@gotplt.org, fberat@redhat.com
Subject: [PATCH v8 1/2] tests: replace read by xread
Date: Wed, 14 Jun 2023 10:52:06 +0200	[thread overview]
Message-ID: <20230614085207.232790-1-fberat@redhat.com> (raw)
In-Reply-To: <20230612151821.199003-2-fberat@redhat.com>

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.
---
Changes since v7:
 - sysdeps/pthread/tst-cancel11.c directly uses read instead of xread to ensure
   clarity on what is actually wanted.

 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 2c9520574d..d6a24f47db 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 1d3d53fb5f..f2e9f263e5 100644
--- a/nptl/tst-cleanup4.c
+++ b/nptl/tst-cleanup4.c
@@ -21,6 +21,8 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#include <support/xunistd.h>
+
 /* 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 8ebe68aa95..5939e53469 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 20ea19af37..788b091ea0 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 0000000000..19f76fcfeb
--- /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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <support/xunistd.h>
+
+#include <support/check.h>
+
+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 43a1e69fcb..0aa2638a8d 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 5df1109dd3..32cf4eb119 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 4dd84d6673..0fdc444154 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 1d5c53049b..93f287aa4f 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 bc4ff308f9..ec8bf0dba3 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 cdadf034cd..0a45f6c5f2 100644
--- a/sysdeps/pthread/tst-fini1mod.c
+++ b/sysdeps/pthread/tst-fini1mod.c
@@ -20,6 +20,8 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#include <support/xunistd.h>
+
 
 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.40.1


  parent reply	other threads:[~2023-06-14  8:52 UTC|newest]

Thread overview: 109+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-18 12:11 [PATCH 0/8] Fix warn unused result Frédéric Bérat
2023-04-18 12:11 ` [PATCH 1/8] tests: fix " Frédéric Bérat
2023-04-18 12:23   ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 07/15] tests: fix warn unused result on asprintf calls Frédéric Bérat
2023-05-25  1:07     ` Siddhesh Poyarekar
2023-05-31 14:36       ` Frederic Berat
2023-04-28 12:21   ` [PATCH v4 08/15] tests: replace write by xwrite Frédéric Bérat
2023-05-25  1:16     ` Siddhesh Poyarekar
2023-06-01 16:39       ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 09/15] tests: replace read by xread Frédéric Bérat
2023-04-28 12:21   ` [PATCH v4 10/15] tests: replace system by xsystem Frédéric Bérat
2023-05-25  1:22     ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 11/15] tests: replace fread by xfread Frédéric Bérat
2023-04-28 12:21   ` [PATCH v4 12/15] tests: replace ftruncate by xftruncate Frédéric Bérat
2023-05-25  1:25     ` Siddhesh Poyarekar
2023-06-01 16:42       ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 13/15] tests: replace fgets by xfgets Frédéric Bérat
2023-04-28 12:21   ` [PATCH v4 14/15] tests: Replace various function calls with their x variant Frédéric Bérat
2023-05-25  1:29     ` Siddhesh Poyarekar
2023-05-25 16:10       ` Frederic Berat
2023-04-28 12:21   ` [PATCH v4 15/15] tests: fix warn unused results Frédéric Bérat
2023-05-25  1:35     ` Siddhesh Poyarekar
2023-06-01 14:27   ` [PATCH v5 04/12] tests: fix warn unused result on asprintf calls Frédéric Bérat
2023-06-01 16:52     ` Siddhesh Poyarekar
2023-06-01 14:27   ` [PATCH v5 07/12] tests: replace system by xsystem Frédéric Bérat
2023-06-01 16:56     ` Siddhesh Poyarekar
2023-06-01 14:27   ` [PATCH v5 12/12] tests: fix warn unused results Frédéric Bérat
2023-06-01 16:57     ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 2/8] catgets/gencat.c: fix warn unused result Frédéric Bérat
2023-04-18 12:36   ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 01/15] " Frédéric Bérat
2023-05-24 22:47     ` Siddhesh Poyarekar
2023-06-01 14:27   ` [PATCH v5 01/12] " Frédéric Bérat
2023-06-01 16:47     ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 3/8] inet/rcmd.c: " Frédéric Bérat
2023-04-18 12:37   ` Siddhesh Poyarekar
2023-04-18 13:40   ` Andreas Schwab
2023-04-18 13:50     ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 4/8] locale/programs/locarchive.c: " Frédéric Bérat
2023-04-18 12:43   ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 02/15] " Frédéric Bérat
2023-05-24 22:49     ` Siddhesh Poyarekar
2023-06-01 14:27   ` [PATCH v5 02/12] malloc/{memusage.c,memusagestat.c}: " Frédéric Bérat
2023-06-01 16:49     ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 5/8] " Frédéric Bérat
2023-04-18 12:47   ` [PATCH 5/8] malloc/{memusage.c, memusagestat.c}: " Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 03/15] malloc/{memusage.c,memusagestat.c}: " Frédéric Bérat
2023-05-25  0:50     ` Siddhesh Poyarekar
2023-06-01 15:55       ` [PATCH] Move {read,write}_all functions to a dedicated header Frédéric Bérat
2023-06-01 17:07         ` Siddhesh Poyarekar
2023-06-02  6:10           ` Frederic Berat
2023-06-02 10:01             ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 6/8] nptl_db/thread_dbP.h: fix warn unused result Frédéric Bérat
2023-04-18 12:49   ` Siddhesh Poyarekar
2023-04-18 12:51     ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 04/15] " Frédéric Bérat
2023-05-25  0:51     ` Siddhesh Poyarekar
2023-05-25  1:52       ` Siddhesh Poyarekar
2023-06-01 14:27   ` [PATCH v5 03/12] " Frédéric Bérat
2023-06-01 16:49     ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 7/8] sunrpc/netname.c: " Frédéric Bérat
2023-04-18 12:51   ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 05/15] " Frédéric Bérat
2023-05-25  0:53     ` Siddhesh Poyarekar
2023-04-18 12:11 ` [PATCH 8/8] sysdeps/pthread/eintr.c: " Frédéric Bérat
2023-04-18 12:54   ` Siddhesh Poyarekar
2023-04-28 12:21   ` [PATCH v4 06/15] " Frédéric Bérat
2023-05-25  0:59     ` Siddhesh Poyarekar
2023-04-28 12:21 ` [PATCH v4 00/15] Fix " Frédéric Bérat
2023-05-25  1:53   ` Siddhesh Poyarekar
2023-06-02 15:28 ` [PATCH v6 0/7] " Frédéric Bérat
2023-06-02 15:28   ` [PATCH v6 1/7] tests: fix warn unused result on asprintf calls Frédéric Bérat
2023-06-06  6:18     ` Siddhesh Poyarekar
2023-06-02 15:28   ` [PATCH v6 2/7] tests: replace read by xread Frédéric Bérat
2023-06-06  6:21     ` Siddhesh Poyarekar
2023-06-06  8:00       ` Frederic Berat
2023-06-12 14:22         ` Siddhesh Poyarekar
2023-06-07 19:04     ` Frederic Berat
2023-06-02 15:28   ` [PATCH v6 3/7] tests: replace system by xsystem Frédéric Bérat
2023-06-06 12:17     ` Siddhesh Poyarekar
2023-06-02 15:28   ` [PATCH v6 4/7] tests: replace fread by xfread Frédéric Bérat
2023-06-06 12:18     ` Siddhesh Poyarekar
2023-06-07 19:03     ` Frederic Berat
2023-06-02 15:28   ` [PATCH v6 5/7] tests: replace fgets by xfgets Frédéric Bérat
2023-06-06 12:20     ` Siddhesh Poyarekar
2023-06-08  5:50     ` Maxim Kuvyrkov
2023-06-08  6:57       ` Frederic Berat
2023-06-02 15:28   ` [PATCH v6 6/7] tests: Replace various function calls with their x variant Frédéric Bérat
2023-06-06 12:20     ` Siddhesh Poyarekar
2023-06-02 15:28   ` [PATCH v6 7/7] Move {read,write}_all functions to a dedicated header Frédéric Bérat
2023-06-06 12:21     ` Siddhesh Poyarekar
2023-06-12 15:18 ` [PATCH v7 0/4] Fix warn unused result Frédéric Bérat
2023-06-12 15:18   ` [PATCH v7 1/4] tests: replace read by xread Frédéric Bérat
2023-06-12 16:57     ` Joseph Myers
2023-06-13 14:22       ` Frederic Berat
2023-06-14  8:52     ` Frédéric Bérat [this message]
2023-06-14 11:51       ` [PATCH v8 1/2] " Siddhesh Poyarekar
2023-06-12 15:18   ` [PATCH v7 2/4] tests: replace system by xsystem Frédéric Bérat
2023-06-13 14:10     ` Siddhesh Poyarekar
2023-06-13 14:13     ` Adhemerval Zanella Netto
2023-06-13 14:16       ` Siddhesh Poyarekar
2023-06-14  8:52     ` [PATCH 2/2] " Frédéric Bérat
2023-06-14 11:53       ` Siddhesh Poyarekar
2023-06-12 15:18   ` [PATCH v7 3/4] tests: replace fread by xfread Frédéric Bérat
2023-06-13 23:57     ` Siddhesh Poyarekar
2023-06-12 15:18   ` [PATCH v7 4/4] tests: replace fgets by xfgets Frédéric Bérat
2023-06-13 12:23     ` Siddhesh Poyarekar
2023-06-13 18:25       ` Joseph Myers
2023-06-13 23:56         ` Siddhesh Poyarekar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230614085207.232790-1-fberat@redhat.com \
    --to=fberat@redhat.com \
    --cc=libc-alpha@sourceware.org \
    --cc=siddhesh@gotplt.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).