Subject: bug#23825: maint: avoid md5sum.c warning from bleeding-edge gcc's -Wstrict-overflow

From: Paul Eggert <[email-masked]>
Date: Thu, 23 Jun 2016 09:13:53 +0200

On 06/22/2016 04:37 PM, Jim Meyering wrote:
> Building with a recent gcc-7 failed, so I wrote the attached patch. I
> think we'll never have 2^31 command line arguments
I would rather fix the code so that it works even if argc is initially 
INT_MAX; the code currently has undefined behavior in that case. I have 
drafted a patch for this (attached; does not have changelog entries yet) 
and will test it a bit. I assume this will fix the warning (don't have 
gcc-7 offhand so can't test this).

As the patch shows, a couple of other coreutils programs have a similar 
bug; I'm a bit surprised gcc-7 didn't complain about them.

Incidentally, 'yes' has a different bug: it mishandles the case where 
'write' succeeds but returns a value less than the buffer size. I'll try 
to look into that too. Simplest would be to use stdio (the comments 
indicate this has performance issues but I don't know what they are, 
anyway correctness trumps performance).
diff --git a/src/md5sum.c b/src/md5sum.c
index 39132e3..3ed1b65 100644
--- a/src/md5sum.c
+++ b/src/md5sum.c
@@ -864,12 +864,13 @@ main (int argc, char **argv)
   if (!O_BINARY && binary < 0)
     binary = 0;
 
+  char **operand_lim = argv + argc;
   if (optind == argc)
-    argv[argc++] = bad_cast ("-");
+    *operand_lim++ = bad_cast ("-");
 
-  for (; optind < argc; ++optind)
+  for (char **operandp = argv + optind; operandp < operand_lim; operandp++)
     {
-      char *file = argv[optind];
+      char *file = *operandp;
 
       if (do_check)
         ok &= digest_check (file);
diff --git a/src/paste.c b/src/paste.c
index bf99fe0..7914aef 100644
--- a/src/paste.c
+++ b/src/paste.c
@@ -465,7 +465,6 @@ int
 main (int argc, char **argv)
 {
   int optc;
-  bool ok;
   char const *delim_arg = "\t";
 
   initialize_main (&argc, &argv);
@@ -505,8 +504,12 @@ main (int argc, char **argv)
         }
     }
 
-  if (optind == argc)
-    argv[argc++] = bad_cast ("-");
+  int nfiles = argc - optind;
+  if (nfiles == 0)
+    {
+      argv[optind] = bad_cast ("-");
+      nfiles++;
+    }
 
   if (collapse_escapes (delim_arg))
     {
@@ -517,10 +520,8 @@ main (int argc, char **argv)
              quotearg_n_style_colon (0, c_maybe_quoting_style, delim_arg));
     }
 
-  if (!serial_merge)
-    ok = paste_parallel (argc - optind, &argv[optind]);
-  else
-    ok = paste_serial (argc - optind, &argv[optind]);
+  bool ok = ((serial_merge ? paste_serial : paste_parallel)
+             (nfiles, &argv[optind]));
 
   free (delims);
 
diff --git a/src/yes.c b/src/yes.c
index 1d2c612..e127310 100644
--- a/src/yes.c
+++ b/src/yes.c
@@ -60,7 +60,6 @@ main (int argc, char **argv)
 {
   char buf[BUFSIZ];
   char *pbuf = buf;
-  int i;
 
   initialize_main (&argc, &argv);
   set_program_name (argv[0]);
@@ -75,24 +74,25 @@ main (int argc, char **argv)
   if (getopt_long (argc, argv, "+", NULL, NULL) != -1)
     usage (EXIT_FAILURE);
 
-  if (argc <= optind)
-    {
-      optind = argc;
-      argv[argc++] = bad_cast ("y");
-    }
+  char **operand_lim = argv + argc;
+  if (optind == argc)
+    *operand_lim++ = bad_cast ("-");
 
   /* Buffer data locally once, rather than having the
      large overhead of stdio buffering each item.  */
-  for (i = optind; i < argc; i++)
+  char **operandp;
+  for (operandp = argv + optind; operandp < operand_lim; operandp++)
     {
-      size_t len = strlen (argv[i]);
+      size_t len = strlen (*operandp);
       if (BUFSIZ < len || BUFSIZ - len <= pbuf - buf)
         break;
-      memcpy (pbuf, argv[i], len);
+      memcpy (pbuf, *operandp, len);
       pbuf += len;
-      *pbuf++ = i == argc - 1 ? '\n' : ' ';
+      *pbuf++ = operandp + 1 == operand_lim ? '\n' : ' ';
     }
-  if (i == argc)
+
+  /* The normal case is to continuously output the local buffer.  */
+  if (operandp == operand_lim)
     {
       size_t line_len = pbuf - buf;
       size_t lines = BUFSIZ / line_len;
@@ -101,32 +101,27 @@ main (int argc, char **argv)
           memcpy (pbuf, pbuf - line_len, line_len);
           pbuf += line_len;
         }
-    }
 
-  /* The normal case is to continuously output the local buffer.  */
-  while (i == argc)
-    {
-      if (write (STDOUT_FILENO, buf, pbuf - buf) == -1)
-        {
-          error (0, errno, _("standard output"));
-          return EXIT_FAILURE;
-        }
+      while (0 <= write (STDOUT_FILENO, buf, pbuf - buf))
+        continue;
+
+      error (0, errno, _("standard output"));
+      return EXIT_FAILURE;
     }
 
   /* If the data doesn't fit in BUFSIZ then output
      what we've buffered, and iterate over the remaining items.  */
-  while (true /* i != argc */)
+  while (true)
     {
-      int j;
       if ((pbuf - buf) && fwrite (buf, pbuf - buf, 1, stdout) != 1)
         {
           error (0, errno, _("standard output"));
           clearerr (stdout);
           return EXIT_FAILURE;
         }
-      for (j = i; j < argc; j++)
-        if (fputs (argv[j], stdout) == EOF
-            || putchar (j == argc - 1 ? '\n' : ' ') == EOF)
+      for (char **trailing = operandp; trailing < operand_lim; trailing++)
+        if (fputs (*trailing, stdout) == EOF
+            || putchar (trailing + 1 == operand_lim ? '\n' : ' ') == EOF)
           {
             error (0, errno, _("standard output"));
             clearerr (stdout);