[Toybox] [landley/toybox] xargs: exec ... Argument list too long (#40)

[Rob Landley]

On 09/28/2017 10:18 AM, enh wrote:
> Did you look at my xargs patch in this thread?

Not closely enough. (Description implied it was the same as last time.)

> It handles all that. I think the find patch is pretty similar, I just
> haven't had time to set up a working test case. (Or rather I fell into
> ratholes while doing so.)

I think this year has been one big rathole for me. :)

Ok, first I should have invoked my "I took too long, apply the submitted
patch as-is, clean it up later" rule a week ago. (It conflicted with
stuff in lib.h and lib.c I was doing, but I've reverted that and can
re-apply after.)

Done and pushed, BUT: I think it's still the wrong fix. I think my
fundamental objection is:

  $ seq 1 200000 | tr '\n' x | xargs
  xargs: argument line too long
  $ echo $(seq 1 200000 | tr '\n' x) | wc
      1       1 1288896

Why does the first _not_ work when the second can? (I note that's the
Ubuntu xargs failing; I think toybox should do better than that.)

You're calling _SC_ARG_MAX which at least on musl returns hardwired
131072, when it should return 1/4 of the stack size limit. (cc'd Rich to
see what he thinks. See the URLs at the end for earlier discussion of
digging into the kernel, the real limit the kernel enforces these days
is 1/4 the stack size ulimit, which defaults to 8388608 on ubuntu 14.04,
1/4 of which is 2 megabytes. I dunno if that's because this netbook has
8 gigs though? It might scale, haven't checked...) Fixing libc to probe
the real size is an acceptable fix, otherwise toybox should do it.

The guaranteed 2k space left over makes sense, although the "can a
single file still advance" issue should be able to cut into that as
necessary. Which would argue for it being PATH_MAX (I.E. the historical
4k) space reserved. But reality can still blow through that, filenames
go arbitrarily deep (or non-filename arguments, as in the above
example). So 2k's no sillier than anything else. :)

And tests/xargs.test needs size tests making sure we calculate the
corner cases exactly. "printf '%0*d' $LEN 0" is probably useful here.
And yes, it works in toybox printf. :)

Rob

P.S. One more FUN corner case is that ulimit -s is a soft limit, the
hard limit is unlimited. So a normal user can ulimit -s 999999999 and
it's accepted. So xargs _could_ finagle this itself if it wanted, at
least for the "one big argument that doesn't fit by itself" case,
although here there probably be dragons. xargs -f anyone? :)

> On Sep 28, 2017 04:41, "Rob Landley" <rob at landley.net
> <mailto:rob at landley.net>> wrote:
> 
>     Sorry, yes I did get time to look at this, I just haven't _fixed_ it
>     yet.
> 
>     The problem is that environment variables and arguments both fit in the
>     same space, which is dynamic at 1/4 the stack size limit as mentioned at
>     http://lists.landley.net/pipermail/toybox-landley.net/2016-August/008592.html
>     <http://lists.landley.net/pipermail/toybox-landley.net/2016-August/008592.html>
>     last time around.
> 
>     Which means that _any_ static size we enforce isn't guaranteed to work,
>     if you have a lot of environment variables defined (or just one really
>     big one). Environment varaibles can even consume the space to the point
>     you can't launch a _single_ argument. (If it's less than PATH_MAX, we
>     have a problem.)
> 
>     So I think it has to probe, and be dynamic, and at least not _loop_ when
>     find produces a single long name that's more than the remaining
>     environment space to launch a child. (I.E. test the corner case that the
>     maximum number of args we can fit is zero.)
> 
>     Space consumption should be exactly sizeof(char *)*(argc+1) +
>     sizeof(char *)+(envc+1) + the strlen+1 of all the argv[] + the strlen+1
>     of all the envp[]. I need to test that's still the case (it used to be
>     when the limit was hardwired 32*4096).
> 
>     Sorry for the delay, still juggling way more things than I have energy
>     for. :(
> 
>     Rob
>