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

[Rob Landley]

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.

There are theoretical rounding errors with small input times (boundary condition
where it averages in an extra zero period at the end it didn't get to write any
data into because it advanced into a new 1/4 second but never read anything
during it), but that's not gonna explain 4x? Hmmm...

> Adding a "*4" in the human_readable call turns 237(-ish)Mb/s into 909(-ish)Mb/s,
> Which is still inaccurate.

Nope, gotta root cause it. If you don't know what went wrong, multiplying the
observed result by a constant to try to approximate the result you expected is
seldom helpful. I want to know WHY the behavior was observed. Causing the
problem to go away without understanding it makes the situation WORSE.

> The below patch shuts the compiler up, but doesn't fix the inaccurate rates

No. The compiler is wrong here. I know how big the strings I fed into it are, I
will -Wno-that-warning in scripts/portability.sh alongside the others if it
can't sanely produce a helpful warning here.

This is "warning: you ever used fgets() at all, I don't care you think you know
what you're doing" level nonsense. When I'm piping data to myself I _can_ know
it will fit. I refuse to defensively program against a broken compiler.
(Especially since this is _exactly_ the sort of thing ASAN is supposedly good at.)

Rob