[Toybox] [PATCH] count.c: Human readable -h option and MAYFORK

Rob Landley rob at landley.net
Tue Oct 17 22:58:41 PDT 2023



On 10/18/23 00:46, Rob Landley wrote:
> On 10/17/23 16:27, Oliver Webb wrote:
>>> Let me know if I screwed stuff up this time, I've been under the weather the
>>> past few days...
>> 
>> I get a -Wformat-overflow warning while compiling this command.
>> Swapping out sprintf with snprintf gives a -Wformat-truncation warning.
> 
> What is the actual warning message? If it's the:
> 
>     sprintf(toybuf+1024, ", %sb, %sb/s, %um%02us", toybuf+256, toybuf+512,
>       seconds/60, seconds%60);
> 
> We have 3k bytes left in the 4k of toybox? Neither of the human_readable() calls
> is going to produce more than a dozen bytes of output and %u maxes out at 4
> billion which is 10 characters. It's 4 arguments for 4 % escapes, argument types
> are correct...
> 
> What is it complaining about, exactly?
> 
> (And what the heck is -Wformat-truncation with snprintf? Isn't that ALLOWED to
> cut the output short? What is it warning _about_?)
> 
>> $ toybox timeout 1 /bin/yes | ./count -l > /dev/null
>> 1243979776 bytes, 1.1Gb, 237Mb/s, 0m01s
>> 
>> In 1 second it copies 1.1Gb of data at 237Mb per second?
> 
> My test was:
> 
>   (for i in $(seq 1 200); do cat README; sleep .1; done; sleep 20) | count -l
> 
> Which alas takes way too long to run for me to put in the test suite but DOES
> insulate the test from obvious variations in the host system.
> 
> I've reproduced your test here, albeit 1.2Mb at 260Kb/s, but it's off by the
> same ratio.

Interesting:

$ timeout 1 yes | ./count -l >/dev/null
Terminated bytes, 2.9Gb, 560Mb/s, 0m01s
$ toybox timeout 1 yes | ./count -l >/dev/null
1238049 bytes, 1.1Mb, 239Kb/s, 0m01s

I.E. "timeout 1 yes | ..." does 3 gigabytes of output in the time toybox does
1.1 megabytes, because toybox timeout is recursively calling toybox yes, which
commit 2c30d4f7a6a6 added line buffering to, so it's doing one write() call per
4 bytes, and the debian version is filling up a much larger output buffer before
flushing it.

I preferred to let libc figure out what sane thing to do with FILE * buffering,
and also mentioned that what I REALLY want is a libc version of the nagle
algorithm here where it collates writes occuring close enough together but
flushes the output when quiescent for long enough that humans would notice
"less" output not updating... but alas I glibc is insane, musl is simplistic,
and I doubt bionic even tried here, so Elliott added micromanagement to yes,
which his comment says it was even more wrong before.

I'm not going to try to fix it now, but I just want to say that TOYFLAG_LINEBUF
clearly isn't the right answer either...

Also... adding LINEBUF when the calls are xprintf() and xputc()... those are
explicitly supposed to flush. Flushing and checking errors are what those DO. If
we want to change so it checks errors without flushing, we can just use printf()
and fputc(c, stdout) directly and put an ferror(stdout) in the yes.c loop?

I'm confused about who wants what here...

Rob


More information about the Toybox mailing list