[v4,14/15] tests: Replace various function calls with their x variant

Message ID 20230428122142.928135-15-fberat@redhat.com
State Superseded
Delegated to: Siddhesh Poyarekar
Headers
Series Fix warn unused result |

Commit Message

Frederic Berat April 28, 2023, 12:21 p.m. UTC
  With fortification enabled, few function calls return result need to be
checked, has they get the __wur macro enabled.
---
 misc/tst-error1.c    | 2 +-
 nss/tst-reload2.c    | 6 +++++-
 posix/tst-chmod.c    | 9 +++++++--
 posix/wordexp-test.c | 6 +++++-
 stdlib/test-canon.c  | 7 ++++++-
 5 files changed, 24 insertions(+), 6 deletions(-)
  

Comments

Siddhesh Poyarekar May 25, 2023, 1:29 a.m. UTC | #1
On 2023-04-28 08:21, Frédéric Bérat wrote:
> With fortification enabled, few function calls return result need to be
> checked, has they get the __wur macro enabled.
> ---
>   misc/tst-error1.c    | 2 +-
>   nss/tst-reload2.c    | 6 +++++-
>   posix/tst-chmod.c    | 9 +++++++--
>   posix/wordexp-test.c | 6 +++++-
>   stdlib/test-canon.c  | 7 ++++++-
>   5 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/misc/tst-error1.c b/misc/tst-error1.c
> index 9c4a62fbd0..65e3fd0c0e 100644
> --- a/misc/tst-error1.c
> +++ b/misc/tst-error1.c
> @@ -9,7 +9,7 @@ static int
>   do_test (int argc, char *argv[])
>   {
>     mtrace ();
> -  (void) freopen (argc == 1 ? "/dev/stdout" : argv[1], "a", stderr);
> +  if (freopen (argc == 1 ? "/dev/stdout" : argv[1], "a", stderr)) {}

There is an xfreopen in support/

>     /* Orient the stream.  */
>     fwprintf (stderr, L"hello world\n");
>     char buf[20000];
> diff --git a/nss/tst-reload2.c b/nss/tst-reload2.c
> index ba9b5b7687..b25e5e3528 100644
> --- a/nss/tst-reload2.c
> +++ b/nss/tst-reload2.c
> @@ -121,7 +121,11 @@ do_test (void)
>     /* Change the root dir.  */
>   
>     TEST_VERIFY (chroot ("/subdir") == 0);
> -  chdir ("/");
> +  if (chdir ("/") < 0)

There is an xchdir in support/

> +    {
> +      printf("Failed to change directory: %m");
> +      return 1;
> +    }
>   
>     /* Check we're NOT using the "inner" nsswitch.conf.  */
>   
> diff --git a/posix/tst-chmod.c b/posix/tst-chmod.c
> index b98a05a265..bec2d2b8eb 100644
> --- a/posix/tst-chmod.c
> +++ b/posix/tst-chmod.c
> @@ -229,7 +229,12 @@ do_test (int argc, char *argv[])
>     close (fd);
>   
>     snprintf (buf, buflen, "%s/..", testdir);
> -  chdir (buf);
> +  if (chdir (buf))
> +    {
> +      printf ("cannot change directory: %m\n");
> +      result = 1;
> +      goto fail;
> +    }
>     /* We are now in the directory above the one we create the test
>        directory in.  */
>   
> @@ -349,7 +354,7 @@ do_test (int argc, char *argv[])
>       }
>   
>    fail:
> -  chdir (startdir);
> +  if (chdir (startdir)) {}
>   
>     /* Remove all the files.  */
>     chmod (testdir, 0700);
> diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
> index bae27d6cee..87d537c931 100644
> --- a/posix/wordexp-test.c
> +++ b/posix/wordexp-test.c
> @@ -332,7 +332,11 @@ do_test (int argc, char *argv[])
>     if (cwd == NULL)
>       cwd = "..";
>   
> -  chdir (cwd);
> +  if (chdir (cwd) < 0)
> +    {
> +      printf ("failed to change dir: %m");
> +      return 1;
> +    }
>     rmdir (tmpdir);
>   
>     return 0;
> diff --git a/stdlib/test-canon.c b/stdlib/test-canon.c
> index 4edee73dd8..5a2e7e1e6e 100644
> --- a/stdlib/test-canon.c
> +++ b/stdlib/test-canon.c
> @@ -154,7 +154,12 @@ do_test (int argc, char ** argv)
>       }
>   
>     for (i = 0; i < (int) (sizeof (symlinks) / sizeof (symlinks[0])); ++i)
> -    symlink (symlinks[i].value, symlinks[i].name);
> +    if (symlink (symlinks[i].value, symlinks[i].name))

There is an xsymlink in support/

> +      {
> +        printf ("%s: Unable to create symlink for %s -> %s\n",
> +		argv[0], symlinks[i].name, symlinks[i].value);
> +        ++errors;
> +      }
>   
>     int has_dir = mkdir ("doesExist", 0777) == 0;
>
  
Frederic Berat May 25, 2023, 4:10 p.m. UTC | #2
It looks like I didn't actually send the changes mentioned in the
title, I may have messed with my patches on this one.

On Thu, May 25, 2023 at 3:29 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
>
>
> On 2023-04-28 08:21, Frédéric Bérat wrote:
> > With fortification enabled, few function calls return result need to be
> > checked, has they get the __wur macro enabled.
> > ---
> >   misc/tst-error1.c    | 2 +-
> >   nss/tst-reload2.c    | 6 +++++-
> >   posix/tst-chmod.c    | 9 +++++++--
> >   posix/wordexp-test.c | 6 +++++-
> >   stdlib/test-canon.c  | 7 ++++++-
> >   5 files changed, 24 insertions(+), 6 deletions(-)
> >
> > diff --git a/misc/tst-error1.c b/misc/tst-error1.c
> > index 9c4a62fbd0..65e3fd0c0e 100644
> > --- a/misc/tst-error1.c
> > +++ b/misc/tst-error1.c
> > @@ -9,7 +9,7 @@ static int
> >   do_test (int argc, char *argv[])
> >   {
> >     mtrace ();
> > -  (void) freopen (argc == 1 ? "/dev/stdout" : argv[1], "a", stderr);
> > +  if (freopen (argc == 1 ? "/dev/stdout" : argv[1], "a", stderr)) {}
>
> There is an xfreopen in support/
>
> >     /* Orient the stream.  */
> >     fwprintf (stderr, L"hello world\n");
> >     char buf[20000];
> > diff --git a/nss/tst-reload2.c b/nss/tst-reload2.c
> > index ba9b5b7687..b25e5e3528 100644
> > --- a/nss/tst-reload2.c
> > +++ b/nss/tst-reload2.c
> > @@ -121,7 +121,11 @@ do_test (void)
> >     /* Change the root dir.  */
> >
> >     TEST_VERIFY (chroot ("/subdir") == 0);
> > -  chdir ("/");
> > +  if (chdir ("/") < 0)
>
> There is an xchdir in support/
>
> > +    {
> > +      printf("Failed to change directory: %m");
> > +      return 1;
> > +    }
> >
> >     /* Check we're NOT using the "inner" nsswitch.conf.  */
> >
> > diff --git a/posix/tst-chmod.c b/posix/tst-chmod.c
> > index b98a05a265..bec2d2b8eb 100644
> > --- a/posix/tst-chmod.c
> > +++ b/posix/tst-chmod.c
> > @@ -229,7 +229,12 @@ do_test (int argc, char *argv[])
> >     close (fd);
> >
> >     snprintf (buf, buflen, "%s/..", testdir);
> > -  chdir (buf);
> > +  if (chdir (buf))
> > +    {
> > +      printf ("cannot change directory: %m\n");
> > +      result = 1;
> > +      goto fail;
> > +    }
> >     /* We are now in the directory above the one we create the test
> >        directory in.  */
> >
> > @@ -349,7 +354,7 @@ do_test (int argc, char *argv[])
> >       }
> >
> >    fail:
> > -  chdir (startdir);
> > +  if (chdir (startdir)) {}
> >
> >     /* Remove all the files.  */
> >     chmod (testdir, 0700);
> > diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
> > index bae27d6cee..87d537c931 100644
> > --- a/posix/wordexp-test.c
> > +++ b/posix/wordexp-test.c
> > @@ -332,7 +332,11 @@ do_test (int argc, char *argv[])
> >     if (cwd == NULL)
> >       cwd = "..";
> >
> > -  chdir (cwd);
> > +  if (chdir (cwd) < 0)
> > +    {
> > +      printf ("failed to change dir: %m");
> > +      return 1;
> > +    }
> >     rmdir (tmpdir);
> >
> >     return 0;
> > diff --git a/stdlib/test-canon.c b/stdlib/test-canon.c
> > index 4edee73dd8..5a2e7e1e6e 100644
> > --- a/stdlib/test-canon.c
> > +++ b/stdlib/test-canon.c
> > @@ -154,7 +154,12 @@ do_test (int argc, char ** argv)
> >       }
> >
> >     for (i = 0; i < (int) (sizeof (symlinks) / sizeof (symlinks[0])); ++i)
> > -    symlink (symlinks[i].value, symlinks[i].name);
> > +    if (symlink (symlinks[i].value, symlinks[i].name))
>
> There is an xsymlink in support/
>
> > +      {
> > +        printf ("%s: Unable to create symlink for %s -> %s\n",
> > +             argv[0], symlinks[i].name, symlinks[i].value);
> > +        ++errors;
> > +      }
> >
> >     int has_dir = mkdir ("doesExist", 0777) == 0;
> >
>
  

Patch

diff --git a/misc/tst-error1.c b/misc/tst-error1.c
index 9c4a62fbd0..65e3fd0c0e 100644
--- a/misc/tst-error1.c
+++ b/misc/tst-error1.c
@@ -9,7 +9,7 @@  static int
 do_test (int argc, char *argv[])
 {
   mtrace ();
-  (void) freopen (argc == 1 ? "/dev/stdout" : argv[1], "a", stderr);
+  if (freopen (argc == 1 ? "/dev/stdout" : argv[1], "a", stderr)) {}
   /* Orient the stream.  */
   fwprintf (stderr, L"hello world\n");
   char buf[20000];
diff --git a/nss/tst-reload2.c b/nss/tst-reload2.c
index ba9b5b7687..b25e5e3528 100644
--- a/nss/tst-reload2.c
+++ b/nss/tst-reload2.c
@@ -121,7 +121,11 @@  do_test (void)
   /* Change the root dir.  */
 
   TEST_VERIFY (chroot ("/subdir") == 0);
-  chdir ("/");
+  if (chdir ("/") < 0)
+    {
+      printf("Failed to change directory: %m");
+      return 1;
+    }
 
   /* Check we're NOT using the "inner" nsswitch.conf.  */
 
diff --git a/posix/tst-chmod.c b/posix/tst-chmod.c
index b98a05a265..bec2d2b8eb 100644
--- a/posix/tst-chmod.c
+++ b/posix/tst-chmod.c
@@ -229,7 +229,12 @@  do_test (int argc, char *argv[])
   close (fd);
 
   snprintf (buf, buflen, "%s/..", testdir);
-  chdir (buf);
+  if (chdir (buf))
+    {
+      printf ("cannot change directory: %m\n");
+      result = 1;
+      goto fail;
+    }
   /* We are now in the directory above the one we create the test
      directory in.  */
 
@@ -349,7 +354,7 @@  do_test (int argc, char *argv[])
     }
 
  fail:
-  chdir (startdir);
+  if (chdir (startdir)) {}
 
   /* Remove all the files.  */
   chmod (testdir, 0700);
diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
index bae27d6cee..87d537c931 100644
--- a/posix/wordexp-test.c
+++ b/posix/wordexp-test.c
@@ -332,7 +332,11 @@  do_test (int argc, char *argv[])
   if (cwd == NULL)
     cwd = "..";
 
-  chdir (cwd);
+  if (chdir (cwd) < 0)
+    {
+      printf ("failed to change dir: %m");
+      return 1;
+    }
   rmdir (tmpdir);
 
   return 0;
diff --git a/stdlib/test-canon.c b/stdlib/test-canon.c
index 4edee73dd8..5a2e7e1e6e 100644
--- a/stdlib/test-canon.c
+++ b/stdlib/test-canon.c
@@ -154,7 +154,12 @@  do_test (int argc, char ** argv)
     }
 
   for (i = 0; i < (int) (sizeof (symlinks) / sizeof (symlinks[0])); ++i)
-    symlink (symlinks[i].value, symlinks[i].name);
+    if (symlink (symlinks[i].value, symlinks[i].name))
+      {
+        printf ("%s: Unable to create symlink for %s -> %s\n",
+		argv[0], symlinks[i].name, symlinks[i].value);
+        ++errors;
+      }
 
   int has_dir = mkdir ("doesExist", 0777) == 0;