On Tue, Oct 17, 2023 at 10:42 PM Rob Landley <[email protected]> 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?
it has no idea what the maximum length of human_readable() output could be. > (And what the heck is -Wformat-truncation with snprintf? Isn't that ALLOWED to > cut the output short? What is it warning _about_?) as long as you check the result, i don't think it cares what you do --- you get the warning when (a) you don't check and (b) it can't prove there's no overflow. > > $ 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.) no, because -- as you said earlier -- snprintf() will truncate. it's asking for proof that that either can't happen, or that you're going to do something sensible if it does. > Rob > _______________________________________________ > Toybox mailing list > [email protected] > http://lists.landley.net/listinfo.cgi/toybox-landley.net _______________________________________________ Toybox mailing list [email protected] http://lists.landley.net/listinfo.cgi/toybox-landley.net
