Subject: Re: bug#21611: probable bug in tee: tee overwrites argv[argc] (coreutils-8.24)
From: Paul Eggert <[email-masked]>
To: Rainer Deyke <[email-masked]>, [email-masked]
Date: Sat, 3 Oct 2015 17:06:52 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Rainer Deyke wrote:
> As mentioned in a comment on line 198 of 'tee.c', 'tee' overwrites
> 'argv[argc]'.  This is very bad style at best, and probably undefined behavior
> on at least some platforms.

Thanks for the report.  It's well-defined behavior on any platform conforming to 
the C standard, so it should be quite safe.  That being said, it's a bit faster 
to subtract 1 from a pointer than to do the equivalent of a memmove of an array, 
so it sounds a bit faster to do things the way you suggest.  Cleaner, too.  I 
installed the attached patch.
From 50c41891523e079f87ba2849b0b47c5c9eb3a3ca Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sat, 3 Oct 2015 17:01:32 -0700
Subject: [PATCH] tee: simplify argv handling

* src/tee.c (tee_files): Last arg is now char ** instead of char
const **, as that is a bit simpler.  All callers changed.  Modify
files[-1], not files[nfiles], as that is a bit faster and simpler.
Latter problem pointed out by Rainer Deyke in:
http://bugs.gnu.org/21611
---
 src/tee.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/src/tee.c b/src/tee.c
index 35120f8..350a201 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -36,7 +36,7 @@
   proper_name ("Richard M. Stallman"), \
   proper_name ("David MacKenzie")
 
-static bool tee_files (int nfiles, const char **files);
+static bool tee_files (int nfiles, char **files);
 
 /* If true, append to output files rather than truncating them. */
 static bool append;
@@ -168,7 +168,7 @@ main (int argc, char **argv)
   /* Do *not* warn if tee is given no file arguments.
      POSIX requires that it work when given no arguments.  */
 
-  ok = tee_files (argc - optind, (const char **) &argv[optind]);
+  ok = tee_files (argc - optind, &argv[optind]);
   if (close (STDIN_FILENO) != 0)
     error (EXIT_FAILURE, errno, _("standard input"));
 
@@ -176,11 +176,11 @@ main (int argc, char **argv)
 }
 
 /* Copy the standard input into each of the NFILES files in FILES
-   and into the standard output.
+   and into the standard output.  As a side effect, modify FILES[-1].
    Return true if successful.  */
 
 static bool
-tee_files (int nfiles, const char **files)
+tee_files (int nfiles, char **files)
 {
   size_t n_outputs = 0;
   FILE **descriptors;
@@ -193,13 +193,6 @@ tee_files (int nfiles, const char **files)
      ? (append ? "ab" : "wb")
      : (append ? "a" : "w"));
 
-  descriptors = xnmalloc (nfiles + 1, sizeof *descriptors);
-
-  /* Move all the names 'up' one in the argv array to make room for
-     the entry for standard output.  This writes into argv[argc].  */
-  for (i = nfiles; i >= 1; i--)
-    files[i] = files[i - 1];
-
   if (O_BINARY && ! isatty (STDIN_FILENO))
     xfreopen (NULL, "rb", stdin);
   if (O_BINARY && ! isatty (STDOUT_FILENO))
@@ -207,10 +200,13 @@ tee_files (int nfiles, const char **files)
 
   fadvise (stdin, FADVISE_SEQUENTIAL);
 
-  /* In the array of NFILES + 1 descriptors, make
-     the first one correspond to standard output.   */
+  /* Set up FILES[0 .. NFILES] and DESCRIPTORS[0 .. NFILES].
+     In both arrays, entry 0 corresponds to standard output.  */
+
+  descriptors = xnmalloc (nfiles + 1, sizeof *descriptors);
+  files--;
   descriptors[0] = stdout;
-  files[0] = _("standard output");
+  files[0] = bad_cast (_("standard output"));
   setvbuf (stdout, NULL, _IONBF, 0);
   n_outputs++;
 
-- 
2.1.0