Re: Options --enable{,-no}-install-program

[Jim Meyering]

Eric Blake <address@hidden> wrote:

> According to Jim Meyering on 1/26/2008 3:55 PM:
>
> | +  "groups -- foo" no longer generates a spurious error about the
> | +  nonexistent group "--".
>
> Huh?  That's been fixed for some time now (Sept 2006):
> http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=b7c315c

Thanks.  I've removed that from NEWS.

> | - while I know too well how bad stdio is, I'm tempted to remove the
> |     new tests of each printf return value.  I noticed that you check
> |     those while not checking putchar.
>
> That's where the combination of xprintf and close-stdout will help -
> xprintf catches any errors that don't set the ferror bit, and all
> remaining errors (all putchar errors, and most printf errors) touch the
> ferror bit and are then caught by close-stdout.  In other words,
...
Yes, that was next up.
However, I'm not yet using xprintf here, and may not do it at all.

I've done this (to be merged into James' patch):

        Don't test printf and putchar calls for failure.

---
 src/group-list.c |   16 ++--------------
 src/groups.c     |   25 +++----------------------
 2 files changed, 5 insertions(+), 36 deletions(-)

diff --git a/src/group-list.c b/src/group-list.c
index e788f8e..ace4908 100644
--- a/src/group-list.c
+++ b/src/group-list.c
@@ -109,21 +109,9 @@ print_group (gid_t gid, bool use_name)
     }

   if (grp == NULL)
-    {
-      if (printf ("%lu", (unsigned long int) gid) < 0)
-        {
-          error (0, errno, _("write error"));
-          ok = false;
-        }
-    }
+    printf ("%lu", (unsigned long int) gid);
   else
-    {
-      if (printf ("%s", grp->gr_name) < 0)
-        {
-          error (0, errno, _("write error"));
-          ok = false;
-        }
-    }
+    printf ("%s", grp->gr_name);
   return ok;
 }

diff --git a/src/groups.c b/src/groups.c
index 8a4673c..baac7b9 100644
--- a/src/groups.c
+++ b/src/groups.c
@@ -65,13 +65,6 @@ the current process (which is different if the groups 
database has changed).\n")
   exit (status);
 }

-static void
-write_error (void)
-{
-  error (0, errno, _("write error"));
-}
-
-
 int
 main (int argc, char **argv)
 {
@@ -111,11 +104,7 @@ main (int argc, char **argv)

       if (!print_group_list (NULL, ruid, rgid, egid, true))
         ok = false;
-      if (EOF == putchar ('\n'))
-        {
-          write_error ();
-          ok = false;
-        }
+      putchar ('\n');
     }
   else
     {
@@ -128,18 +117,10 @@ main (int argc, char **argv)
           ruid = pwd->pw_uid;
           rgid = egid = pwd->pw_gid;

-          if (printf ("%s : ", argv[optind]) < 0)
-            {
-              write_error ();
-              ok = false;
-            }
+          printf ("%s : ", argv[optind]);
           if (!print_group_list (argv[optind++], ruid, rgid, egid, true))
             ok = false;
-          if (EOF == putchar ('\n'))
-            {
-              write_error ();
-              ok = false;
-            }
+          putchar ('\n');
         }
     }

--
1.5.4.rc5

> And the above need only be 'putchar ('\n');' instead of five lines, and
> you don't need the helper function write_error().
>
> | +        }
> | +    }
> | +
> | +  exit (ok ? EXIT_SUCCESS : EXIT_FAILURE);
>
> And if you get here, you can use simply 'exit EXIT_SUCCESS;' - the atexit
> handler will take care of correcting the exit status if there was any
> write error in the meantime.

Almost.  "ok" can be set to false for other reasons.