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

[enh]

On Tue, Oct 17, 2023 at 10:55 PM Rob Landley <rob at landley.net> wrote:
>
>
>
> 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,

you _say_ that, but that's not what you _do_ --- you always tell libc
either "line buffering" or "no buffering":
```
setvbuf(stdout, 0, (which->flags & TOYFLAG_LINEBUF) ? _IOLBF : _IONBF, 0);
```

if you had
```
if (which->flags & TOYFLAG_LINEBUF) setvbuf(stdout, 0, _IOLBF, 0);
```
we'd argue a lot less about buffering :-)

i also wonder whether
```
if (isatty() || (which->flags & TOYFLAG_LINEBUF)) setvbuf(stdout, 0, _IOLBF, 0);
```
would make both of us happier?

> 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.

if you were on WG14, you could suggest adding that kind of buffering :-)

as it is, it's a bit hard for any libc to say "i know what you _told_
me, but i'm going to do my own unspecified thing that there's no way
to explicitly ask for".

also: rob landley asking for libc to spawn a stdio stream flushing thread?! :-P

> 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
> _______________________________________________
> Toybox mailing list
> Toybox at lists.landley.net
> http://lists.landley.net/listinfo.cgi/toybox-landley.net