[Toybox] [PATCH] xargs: avoid "Argument list too long".
rob at landley.net
Fri Oct 25 00:08:15 PDT 2019
On 10/24/19 7:53 PM, enh wrote:
> i haven't tested my other cases, but your patch doesn't address the -0 issues:
How many cases do you have?
I hadn't noticed the second test _wasn't_ commented out, and there wasn't a
change to the actual -0 accounting code. (Issues plural? Is there more than one
problem with -0?)
> /tmp/toybox$ ./toybox find / -print0 2>/dev/null | ./toybox xargs -0 > /dev/nul
> l ; echo $?
> xargs: exec echo: Argument list too long
Reproduced with /usr on devuan. (It invokes echo fine the first time, but the
second goes boing.)
bytes=2092116 limit=2092178 entries=38357 : success
bytes=2092153 limit=2092178 entries=34423 : failure
An accounting difference of 35 bytes between success and failure. And the
failing one was significantly _fewer_ entries so it's not "we missed some
per-entry resource that added up to kill us", it's _fewer_ entries.
No idea what's going on there. The -0 case was _always_ adding the sizeof(char
*) and NUL terminator. Hmmm, there should be an extra char * in both
environ_bytes and argv (the NULL terminating entry each time) but that's a
constant 16 overshoot into the 2048 fudge factor. I stuck a printf in and it
_thinks_ it's calling exec about 25 bytes under budget in the case that fails?
Hmmm... Why would this ONLY hit the -0 case and not the other one? By the time
we exec() it shouldn't matter?
I'm guessing your "fix" was this bit?
- // though obviously that's not really something you can guarantee.
- bytes = sysconf(_SC_ARG_MAX) - environ_bytes() - 2048;
- if (!TT.s || TT.s > bytes) TT.s = bytes;
+ // though obviously that's not really something you can guarantee. In
+ // practice on Linux we need to stay clear by a page (rounding?).
+ TT.limit = sysconf(_SC_ARG_MAX) - environ_bytes() - sysconf(_SC_PAGESIZE);
That really sounds like just throwing a larger fudge factor at it rather than
understanding what's going on.
The thing is, we're going ~17 bytes into the earlier fudge factor (there's a
NULL pointer each at the end of argc and envp that aren't accounted for, and
then we start the command size calculation with -1 to get the right size for
"xargs -s 8" and such) and it seems to have worked?
Your change implies we need to avoid this last page _entirely_, and the first
invocation worked fine. We've _already_ got a fudge factor of 2048 bytes that's
somehow not enough here, doubling it doesn't fill me with warm fuzzies. If I
don't know why it's breaking, I don't have confidence we fixed it.
Sigh, time to fire up QEMU and stick printk's into the kernel...
More information about the Toybox