> On Jul 22, 2019, at 01:49, Claudio Jeker <[email protected]> wrote:
> >> On Sun, Jul 21, 2019 at 08:30:23AM -0500, Scott Cheloha wrote: >> On Sun, Jul 21, 2019 at 11:49:10AM +0200, Mark Kettenis wrote: >>>> Date: Sun, 21 Jul 2019 10:50:27 +0200 >>>> From: Claudio Jeker <[email protected]> >>>> >>>>> On Sat, Jul 20, 2019 at 08:12:40AM -0500, Scott Cheloha wrote: >>>>> There are cases for ping(8)'s "-i wait" option that we don't handle >>>>> correctly. >>>>> >>>>> Negative values smaller than -1: >>>>> >>>>> $ doas ping -i -0.1 localhost >>>>> [no error] >>>>> >>>>> Positive values less than one microsecond: >>>>> >>>>> $ doas ping -i 0.0000001 localhost >>>>> [no error] >>>>> >>>>> Large positive values that actually fit into a timeval: >>>>> >>>>> $ ping -i $(bc -e '2^63 - 1' -e quit) localhost >>>>> ping: illegal timing interval 9223372036854775807 >>>>> >>>>> The first two cases are bugs in the input checking. The latter case >>>>> is a limitation of IEEE doubles: with only 52 bits of significand you >>>>> can't represent the full range of a timeval on a platform with 64-bit >>>>> time_t. >>>>> >>>>> This patch addresses the first two cases with better error checking. >>>>> It also tries to handle the latter case a bit better by using IEEE >>>>> quads, i.e. long doubles. With 64 bits of significand you can cover >>>>> our time_t and the above case works. It isn't ~perfect~, but it's as >>>>> close as we can get to perfect without parsing the number by hand as >>>>> we do in sleep(1) (cf. bin/sleep/sleep.c). >>>>> >>>>> With this patch those cases all work as expected: >>>>> >>>>> $ doas ping -i -0.1 localhost >>>>> ping: interval is invalid: -0.1 >>>>> $ doas ping -i 0.0000001 localhost >>>>> ping: interval is too small: 0.0000001 >>>>> $ ping -i $(bc -e '2^63' -e quit) localhost >>>>> ping: interval is too large: 9223372036854775808 >>>>> $ ping -i $(bc -e '2^63 - 1' -e quit) localhost >>>>> PING localhost.local (23.195.69.106): 56 data bytes >>>>> 64 bytes from 23.195.69.106: icmp_seq=0 ttl=54 time=18.001 ms >>>>> [stalls forever] >>>>> >>>>> -- >>>>> >>>>> While we're modifying this code, I think "-i interval" is better than >>>>> "-i wait". The "i for interval" mnemonic is particularly nice. The >>>>> current "wait" argument name sort-of implies a relationship with the >>>>> "-w maxwait" option, which is not the case. We're also already using >>>>> "timing interval" in these error messages here. I've tweaked the >>>>> error messages to look more like the usual strtonum(3) error messages. >>>>> >>>>> ok? >>>> >>>> Can't we limit the -i maximum value to something that fits into the double >>>> instead of using long double in ping. Nobody will ever try to using ping >>>> with a timeout that is longer than the operators expected life time. >> >> Sure. Here's a diff that uses UINT_MAX. The oldest known person >> lived to 123. French, if I recall correctly. UINT_MAX gives us 136 >> years, so we have a bit of room to grow just in case someone beats the >> record. And 52 bits of significand is plenty for 32 bits of integral >> plus a fractional portion. >> >>> Using a long double isn't a solution anyway, since we have quite a few >>> architectures where long double is the same as double. >> >> Oh, hmmm, didn't know that. >> >> -- >> >> this diff better? > > Can't this just use the overflow detection of strtod()? You mean the ERANGE thing? That isn't useful here. The normal range for a double is far more vast than what a timeval can represent. We have to check by hand.
