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);
}
