xargs environment size accounting is misplaced (findutils-4.1.7)

[Bob Byrnes]

I am using findutils-4.1.7 (which seems to be missing on alpha.gnu.org,
so I grabbed it from the latest RedHat source RPM).

The code that takes the size of the environment into account is misplaced.
Because it occurs before the command line options are parsed, it always
uses the default value of 20k for -s (or --max-chars).  Hence, xargs always
fails if you have at least 20k in your environment, and increasing the
limit on the command line doesn't help.  In fact, even xargs --help fails
in this case.

Furthermore, if you have a small environment (so the test does not fail)
and you subsequently use -s or --max-chars, then the environment size is
not taken into account for the new value of arg_max.

Here's a short shell script that illustrates the problem:

----------------------------------------
#!/bin/sh
#
# Expose bugs in GNU xargs version 4.1.7:
#
# A kilobyte (64*16) of data, for the environment.
x="xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
k="$x$x$x$x$x$x$x$x$x$x$x$x$x$x$x$x"

# Add at least 20k bytes to the environment.
n=1
while [ $n -le 20 ]
do
    eval export ENV_$n=$k
    n=`expr $n + 1`
done

# This doesn't work, even though there should be plenty of space for the args.
echo Hello world! | xargs -s 40960

# Even this doesn't work, because the environ size check occurs before getopt.
xargs --help
----------------------------------------

I have appended a patch that fixes this, and adds a missing newline at
the end of the usage message.  I have also composed a ChangeLog entry.

Finally, here are a few other items for the TODO list:

    --  The documentation doesn't actually say that the size of the environment
        is taken into account for the max-chars limit.

    --  Not all systems use a single, combined limit for argv + environ; e.g.
        cygwin on Windows seems to use separate limits.  It would be nice if
        xargs would ignore the size of the environment on such systems, where
        it is not relevant.  I guess that should be detected by configure.

    --  The default value for -n (or --max-args) appears to be 1024, but this
        isn't mentioned in the documentation.

    --  There doesn't seem to be any way to remove the max-args limit.  It can
        be set to some high value, of course, but it might be cleaner to make
        the argument optional, and interpret a missing value as "no limit".
        Or a separate --no-max-args option could be used.

Bob Byrnes                        e-mail: address@hidden
Curl Corporation                  phone:  617-761-1200
400 Technology Square, 8th Floor  fax:    617-761-1201
Cambridge, MA  02139

----------------------------------------

2002-09-26  Robert Byrnes <address@hidden>

        * xargs/xargs.c (main): Take the size of the environment into account
        only after parsing command line options.
        (usage): Added newline at end of usage text.

--- xargs/xargs.c.orig  Sun May 20 16:39:38 2001
+++ xargs/xargs.c       Thu Sep 26 20:55:00 2002
@@ -292,11 +292,6 @@ main (int argc, char **argv)
   if (arg_max > 20 * 1024)
     arg_max = 20 * 1024;
 
-  /* Take the size of the environment into account.  */
-  arg_max -= env_size (environ);
-  if (arg_max <= 0)
-    error (1, 0, _("environment is too large for exec"));
-
   while ((optc = getopt_long (argc, argv, "+0e::i::l::n:prs:txP:",
                              longopts, (int *) 0)) != -1)
     {
@@ -377,6 +372,11 @@ main (int argc, char **argv)
        }
     }
 
+  /* Take the size of the environment into account.  */
+  arg_max -= env_size (environ);
+  if (arg_max <= 0)
+    error (1, 0, _("environment is too large for exec"));
+
   if (replace_pat || lines_per_exec)
     exit_if_size_exceeded = true;
 
@@ -922,6 +922,6 @@ usage (FILE *stream, int status)
        [--max-args=max-args] [--no-run-if-empty] [--version] [--help]\n\
        [command [initial-arguments]]\n"),
           program_name);
-  fputs (_("\nReport bugs to <address@hidden>."), stream);
+  fputs (_("\nReport bugs to <address@hidden>.\n"), stream);
   exit (status);
 }