Hi Scott,
Scott Cheloha wrote on Sun, Jun 30, 2019 at 06:49:28AM -0500:
> This is cleaner, shorter.
>
> - Remove the intermediate variables and just build the timespec
> directly.
>
> - Use for-loops to consolidate initialization/incrementation of cp
> into one spot in each loop.
>
> - Use one loop to parse fractions of a second: less duplicate code
> keeps the fraction path shorter. Expand the comment to detail why
> this is fine. We then explain the magic multiplier at the point
> of use instead of making you read the next loop to find out what
> happened in the prior loop.
I like that.
It is easier to understand and survived my code review and testing.
> ok?
OK schwarze@
Ingo
> Index: sleep.c
> ===================================================================
> RCS file: /cvs/src/bin/sleep/sleep.c,v
> retrieving revision 1.27
> diff -u -p -r1.27 sleep.c
> --- sleep.c 10 Jan 2019 16:41:10 -0000 1.27
> +++ sleep.c 30 Jun 2019 11:22:58 -0000
> @@ -49,9 +49,8 @@ int
> main(int argc, char *argv[])
> {
> int ch;
> - time_t secs = 0, t;
> + time_t t;
> char *cp;
> - long nsecs = 0;
> struct timespec rqtp;
> int i;
>
> @@ -71,40 +70,32 @@ main(int argc, char *argv[])
> if (argc != 1)
> usage();
>
> - cp = *argv;
> - while ((*cp != '\0') && (*cp != '.')) {
> + timespecclear(&rqtp);
> +
> + /* Handle whole seconds. */
> + for (cp = *argv; *cp != '\0' && *cp != '.'; cp++) {
> if (!isdigit((unsigned char)*cp))
> errx(1, "seconds is invalid: %s", *argv);
> - t = (secs * 10) + (*cp++ - '0');
> - if (t / 10 != secs) /* oflow */
> + t = (rqtp.tv_sec * 10) + (*cp - '0');
> + if (t / 10 != rqtp.tv_sec) /* overflow */
> errx(1, "seconds is too large: %s", *argv);
> - secs = t;
> + rqtp.tv_sec = t;
> }
>
> - /* Handle fractions of a second */
> + /*
> + * Handle fractions of a second. The multiplier divides to zero
> + * after nine digits so anything more precise than a nanosecond is
> + * validated but not used.
> + */
> if (*cp == '.') {
> - cp++;
> - for (i = 100000000; i > 0; i /= 10) {
> - if (*cp == '\0')
> - break;
> + i = 100000000;
> + for (cp++; *cp != '\0'; cp++) {
> if (!isdigit((unsigned char)*cp))
> errx(1, "seconds is invalid: %s", *argv);
> - nsecs += (*cp++ - '0') * i;
> - }
> -
> - /*
> - * We parse all the way down to nanoseconds
> - * in the above for loop. Be pedantic about
> - * checking the rest of the argument.
> - */
> - while (*cp != '\0') {
> - if (!isdigit((unsigned char)*cp++))
> - errx(1, "seconds is invalid: %s", *argv);
> + rqtp.tv_nsec += (*cp - '0') * i;
> + i /= 10;
> }
> }
> -
> - rqtp.tv_sec = secs;
> - rqtp.tv_nsec = nsecs;
>
> if (timespecisset(&rqtp)) {
> if (nanosleep(&rqtp, NULL) == -1)
