On Sun, Jul 21, 2019 at 08:30:23AM -0500, Scott Cheloha wrote:
> 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.
Looks good.

> @@ -332,17 +332,21 @@ main(int argc, char *argv[])
>               case 'S':       /* deprecated */
>                       source = optarg;
>                       break;
> -             case 'i':               /* wait between sending packets */
> -                     intval = strtod(optarg, &e);
> -                     if (*optarg == '\0' || *e != '\0')
> -                             errx(1, "illegal timing interval %s", optarg);
> -                     if (intval < 1 && ouid)
> -                             errx(1, "only root may use interval < 1s");
> -                     interval.tv_sec = (time_t)intval;
> -                     interval.tv_usec =
> -                         (long)((intval - interval.tv_sec) * 1000000);
> -                     if (interval.tv_sec < 0)
> -                             errx(1, "illegal timing interval %s", optarg);
> +             case 'i':               /* interval between packets */
> +                     seconds = strtod(optarg, &e);
> +                     if (*optarg == '\0' || *e != '\0' || seconds < 0.0)
> +                             errx(1, "interval is invalid: %s", optarg);
> +                     fraction = modf(seconds, &integral);
> +                     if (integral > UINT_MAX)
> +                             errx(1, "interval is too large: %s", optarg);
> +                     interval.tv_sec = integral;
> +                     interval.tv_usec = fraction * 1000000.0;
> +                     if (!timerisset(&interval))
> +                             errx(1, "interval is too small: %s", optarg);
> +                     if (interval.tv_sec < 1 && ouid != 0) {
You can check this earlier against integral directly and avoid handling
`interval' in the non-root case.

> +                             errx(1, "only root may use an interval smaller"
> +                                 "than one second");
> +                     }
>                       options |= F_INTERVAL;
>                       break;
>               case 'L':

> @@ -160,13 +160,15 @@ Set the hoplimit.
>  Specify the interface address to transmit from
>  on machines with multiple interfaces.
>  For unicast and multicast pings.
> -.It Fl i Ar wait
> -Wait
> -.Ar wait
> -seconds between sending each packet.
> -The default is to wait for one second between each packet.
> -The wait time may be fractional, but only the superuser may specify
> -a value less than one second.
> +.It Fl i Ar interval
> +Wait at least
> +.Ar interval
> +seconds between each outgoing packet.
I think the old "between sending each packet" still reads better.

> +The default is one second.
> +The
> +.Ar interval
> +may contain a fractional portion.
> +Only the superuser may specify a value less than one second.
Here, the old sentence combining both statements in one sentence reads
better to me, but no objections.

>  This option is incompatible with the
>  .Fl f
>  option.
> 

Reply via email to