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);
