Roughly top to bottom: - Sort includes alphabetically - Ditch __progname for getprogname(3) - Sort prototypes alphabetically - usage() is __dead - Sort stack variables by size (?), then alphabetically (?) * I have no idea if I did this right, but it looks cleaner than before. * Don't sizes vary by architecture? At least for pointers? * Alphabetically by type name and then by variable name? Some other scheme? style(9) seems to contradict itself here in the example. - rqtp -> timeout, to match nanosleep(2) manpage - t -> tsecs, more obvious - Don't initialize variables in the declaration block - Brace the getopt switch statement - Use for loops in lieu of while loops to initialize and iterate cp * I don't think it clarifies things in the first nanosecond loop, so that's unchanged - Check explicitly for -1 on nanosleep's return - No need to (void) the fprintf in usage() - POSIX.2 was consolidated into POSIX.1 after 1997 - sleep(1) *may* exit 0 when it gets a SIGALRM: it's allowed to do other things, too - No more lint: drop ARGSUSED - _exit(2)ing from a signal handler is (now) a well-known practice, no need to explain
ok? -- Scott Cheloha Index: bin/sleep/sleep.c =================================================================== RCS file: /cvs/src/bin/sleep/sleep.c,v retrieving revision 1.26 diff -u -p -r1.26 sleep.c --- bin/sleep/sleep.c 4 Feb 2018 02:18:15 -0000 1.26 +++ bin/sleep/sleep.c 14 Feb 2018 17:12:52 -0000 @@ -31,52 +31,51 @@ */ #include <ctype.h> +#include <err.h> #include <signal.h> #include <stdio.h> #include <stdlib.h> #include <time.h> #include <unistd.h> -#include <err.h> -extern char *__progname; - -void usage(void); void alarmh(int); +void __dead usage(void); int main(int argc, char *argv[]) { - int ch; - time_t secs = 0, t; + struct timespec timeout; + time_t secs, tsecs; + long nsecs; char *cp; - long nsecs = 0; - struct timespec rqtp; - int i; + int ch, i; + + secs = nsecs = 0; if (pledge("stdio", NULL) == -1) err(1, "pledge"); signal(SIGALRM, alarmh); - while ((ch = getopt(argc, argv, "")) != -1) + while ((ch = getopt(argc, argv, "")) != -1) { switch(ch) { default: usage(); } + } argc -= optind; argv += optind; if (argc != 1) usage(); - cp = *argv; - while ((*cp != '\0') && (*cp != '.')) { + 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 */ + tsecs = (secs * 10) + (*cp - '0'); + if (tsecs / 10 != secs) /* overflow */ errx(1, "seconds is too large: %s", *argv); - secs = t; + secs = tsecs; } /* Handle fractions of a second */ @@ -95,8 +94,8 @@ main(int argc, char *argv[]) * in the above for loop. Be pedantic about * checking the rest of the argument. */ - while (*cp != '\0') { - if (!isdigit((unsigned char)*cp++)) + for (; *cp != '\0'; cp++) { + if (!isdigit((unsigned char)*cp)) errx(1, "seconds is invalid: %s", *argv); } } @@ -108,38 +107,32 @@ main(int argc, char *argv[]) * calls if we have more than that. */ if (secs > 100000000) { - rqtp.tv_sec = 100000000; - rqtp.tv_nsec = 0; + timeout.tv_sec = 100000000; + timeout.tv_nsec = 0; } else { - rqtp.tv_sec = secs; - rqtp.tv_nsec = nsecs; + timeout.tv_sec = secs; + timeout.tv_nsec = nsecs; } - if (nanosleep(&rqtp, NULL)) - err(1, NULL); - secs -= rqtp.tv_sec; - nsecs -= rqtp.tv_nsec; + if (nanosleep(&timeout, NULL) == -1) + err(1, "nanosleep"); + secs -= timeout.tv_sec; + nsecs -= timeout.tv_nsec; } return (0); } -void +void __dead usage(void) { - (void)fprintf(stderr, "usage: %s seconds\n", __progname); + fprintf(stderr, "usage: %s seconds\n", getprogname()); exit(1); } /* - * POSIX 1003.2 says sleep should exit with 0 return code on reception - * of SIGALRM. + * POSIX.1 says sleep may exit with status 0 upon receipt of SIGALRM. */ -/* ARGSUSED */ void alarmh(int signo) { - /* - * exit() flushes stdio buffers, which is not legal in a signal - * handler. Use _exit(). - */ _exit(0); }