Hi,
Scott Cheloha wrote on Thu, Jul 13, 2017 at 07:30:26PM -0500:
> We currently use a mix of perror(3) and err(3).
>
> In one case you can merge perror + exit into err, which is nice.
>
> The warns, though, are not equivalent (you get a "time: " prefix),
> so maybe this is too risky.
>
> Putting it out here anyway.
We can do even better.
* In main(), prefer "return" over exit(3).
* This is ugly and confusing:
$ sh -c 'ulimit -p 29; /usr/bin/time ls'
time: Resource temporarily unavailable
This is easier to understand:
$ sh -c 'ulimit -p 29; /usr/bin/time ls'
time: fork: Resource temporarily unavailable
* In complex situations, this can be confusing:
$ /usr/bin/time foo/bar
foo/bar: No such file or directory
This is easier to understand:
$ /usr/bin/time foo/bar
time: foo/bar: No such file or directory
* This is plain nonsense:
$ ( /usr/bin/time sleep 100 ; echo $? ) & pkill -KILL sleep
Command terminated abnormally.
0.00 real 0.00 user 0.00 sys
signal: Invalid argument
1
signal(3) can only fail for SIGKILL or SIGSTOP (or for an invalid
argument, which cannot happen here). Printing "Invalid argument"
is completely pointless, and confusing.
However, we do want an EXIT STATUS of 128 + 9 in this case:
$ ( /usr/bin/time sleep 100 ; echo $? ) & pkill -KILL sleep
Command terminated abnormally.
0.00 real 0.00 user 0.00 sys
137
To wit, look at the builtin:
$ ( time sleep 100 ; echo $? ) & pkill -KILL sleep
Killed
0m00.00s real 0m00.00s user 0m00.00s system
137
OK?
Ingo
Index: time.c
===================================================================
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.24
diff -u -p -r1.24 time.c
--- time.c 22 Jul 2017 17:01:09 -0000 1.24
+++ time.c 22 Jul 2017 17:43:57 -0000
@@ -67,7 +67,6 @@ main(int argc, char *argv[])
break;
default:
usage();
- /* NOTREACHED */
}
}
@@ -80,14 +79,12 @@ main(int argc, char *argv[])
clock_gettime(CLOCK_MONOTONIC, &before);
switch(pid = vfork()) {
case -1: /* error */
- perror("time");
- exit(1);
- /* NOTREACHED */
+ warn("fork");
+ return 1;
case 0: /* child */
execvp(*argv, argv);
- perror(*argv);
+ warn("%s", *argv);
_exit((errno == ENOENT) ? 127 : 126);
- /* NOTREACHED */
}
/* parent */
@@ -168,11 +165,11 @@ main(int argc, char *argv[])
if (exitonsig) {
if (signal(exitonsig, SIG_DFL) == SIG_ERR)
- perror("signal");
+ return 128 + exitonsig;
else
kill(getpid(), exitonsig);
}
- exit(WIFEXITED(status) ? WEXITSTATUS(status) : EXIT_FAILURE);
+ return WIFEXITED(status) ? WEXITSTATUS(status) : EXIT_FAILURE;
}
__dead void
