Hi,

The "real" elapsed time for time(1) and the ksh/csh time builtins is
currently computed with gettimeofday(2), so it's subject to changes
by adjtime(2) and, if you're really unlucky, clock_settime(2) or
settimeofday(2).  In pathological cases you can get negative values
in the output.

This seems wrong to me.  I personally use these tools like a stopwatch,
and I was surprised to see that the elapsed difference wasn't (more)
immune to changes to the system clock.

The attached patches change the "real" listing for time(1), ksh's time
builtin, and csh's time builtin to use a monotonic clock, which I think
more closely matches what the typical user and programmer expects.  This
interpretation is, near as I can tell, also compatible with the POSIX.1
2008 description of the time(1) utility.  In particular, the use of
"elapsed," implying a scalar value, makes me think that this is the
intended behavior. [1]

NetBSD did this in 2011 without much fanfare, though for some reason they
did it for time(1) and csh's builtin but not for ksh's builtin. [2]

I've tested pathological cases in each of the three and these patches
correct the result in said cases without (perceptibly) changing the
result in the typical case.

Thoughts?  Feedback?

--
Scott Cheloha

[1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/time.html
[2] http://gnats.netbsd.org/45592

Index: bin/csh/csh.h
===================================================================
RCS file: /cvs/src/bin/csh/csh.h,v
retrieving revision 1.28
diff -u -p -r1.28 csh.h
--- bin/csh/csh.h       26 Dec 2015 13:48:38 -0000      1.28
+++ bin/csh/csh.h       12 Jul 2017 04:15:04 -0000
@@ -122,7 +122,7 @@ char   *seterr;                     /* Error message from 
 #include <sys/time.h>
 #include <sys/resource.h>
 
-struct timeval time0;          /* Time at which the shell started */
+struct timespec time0;         /* Time at which the shell started */
 struct rusage ru0;
 
 /*
Index: bin/csh/extern.h
===================================================================
RCS file: /cvs/src/bin/csh/extern.h,v
retrieving revision 1.25
diff -u -p -r1.25 extern.h
--- bin/csh/extern.h    26 Dec 2015 13:48:38 -0000      1.25
+++ bin/csh/extern.h    12 Jul 2017 04:15:04 -0000
@@ -272,7 +272,7 @@ void         plist(struct varent *);
 void   donice(Char **, struct command *);
 void   dotime(Char **, struct command *);
 void   prusage(struct rusage *, struct rusage *,
-           struct timeval *, struct timeval *);
+           struct timespec *, struct timespec *);
 void   ruadd(struct rusage *, struct rusage *);
 void   settimes(void);
 void   pcsecs(long);
Index: bin/csh/proc.c
===================================================================
RCS file: /cvs/src/bin/csh/proc.c,v
retrieving revision 1.30
diff -u -p -r1.30 proc.c
--- bin/csh/proc.c      26 Dec 2015 13:48:38 -0000      1.30
+++ bin/csh/proc.c      12 Jul 2017 04:15:04 -0000
@@ -108,7 +108,7 @@ found:
     }
     else {
        if (pp->p_flags & (PTIME | PPTIME) || adrof(STRtime))
-           (void) gettimeofday(&pp->p_etime, NULL);
+           (void) clock_gettime(CLOCK_MONOTONIC, &pp->p_etime);
 
        pp->p_rusage = ru;
        if (WIFSIGNALED(w)) {
@@ -494,7 +494,7 @@ palloc(int pid, struct command *t)
     }
     pp->p_next = proclist.p_next;
     proclist.p_next = pp;
-    (void) gettimeofday(&pp->p_btime, NULL);
+    (void) clock_gettime(CLOCK_MONOTONIC, &pp->p_btime);
 }
 
 static void
@@ -799,8 +799,8 @@ prcomd:
 static void
 ptprint(struct process *tp)
 {
-    struct timeval tetime, diff;
-    static struct timeval ztime;
+    struct timespec tetime, diff;
+    static struct timespec ztime;
     struct rusage ru;
     static struct rusage zru;
     struct process *pp = tp;
@@ -809,8 +809,8 @@ ptprint(struct process *tp)
     tetime = ztime;
     do {
        ruadd(&ru, &pp->p_rusage);
-       timersub(&pp->p_etime, &pp->p_btime, &diff);
-       if (timercmp(&diff, &tetime, >))
+       timespecsub(&pp->p_etime, &pp->p_btime, &diff);
+       if (timespeccmp(&diff, &tetime, >))
            tetime = diff;
     } while ((pp = pp->p_friends) != tp);
     prusage(&zru, &ru, &tetime, &ztime);
Index: bin/csh/proc.h
===================================================================
RCS file: /cvs/src/bin/csh/proc.h,v
retrieving revision 1.3
diff -u -p -r1.3 proc.h
--- bin/csh/proc.h      2 Jun 2003 23:32:07 -0000       1.3
+++ bin/csh/proc.h      12 Jul 2017 04:15:04 -0000
@@ -50,8 +50,8 @@ struct process {
     pid_t   p_pid;
     pid_t   p_jobid;           /* pid of job leader */
     /* if a job is stopped/background p_jobid gives its pgrp */
-    struct timeval p_btime;    /* begin time */
-    struct timeval p_etime;    /* end time */
+    struct timespec p_btime;   /* begin time */
+    struct timespec p_etime;   /* end time */
     struct rusage p_rusage;
     Char   *p_command;         /* first PMAXLEN chars of command */
 };
Index: bin/csh/time.c
===================================================================
RCS file: /cvs/src/bin/csh/time.c,v
retrieving revision 1.14
diff -u -p -r1.14 time.c
--- bin/csh/time.c      22 Aug 2013 04:43:40 -0000      1.14
+++ bin/csh/time.c      12 Jul 2017 04:15:04 -0000
@@ -46,7 +46,7 @@ settimes(void)
 {
     struct rusage ruch;
 
-    (void) gettimeofday(&time0, NULL);
+    (void) clock_gettime(CLOCK_MONOTONIC, &time0);
     (void) getrusage(RUSAGE_SELF, &ru0);
     (void) getrusage(RUSAGE_CHILDREN, &ruch);
     ruadd(&ru0, &ruch);
@@ -60,13 +60,13 @@ void
 /*ARGSUSED*/
 dotime(Char **v, struct command *t)
 {
-    struct timeval timedol;
+    struct timespec timedol;
     struct rusage ru1, ruch;
 
     (void) getrusage(RUSAGE_SELF, &ru1);
     (void) getrusage(RUSAGE_CHILDREN, &ruch);
     ruadd(&ru1, &ruch);
-    (void) gettimeofday(&timedol, NULL);
+    (void) clock_gettime(CLOCK_MONOTONIC, &timedol);
     prusage(&ru0, &ru1, &timedol, &time0);
 }
 
@@ -112,8 +112,8 @@ ruadd(struct rusage *ru, struct rusage *
 }
 
 void
-prusage(struct rusage *r0, struct rusage *r1, struct timeval *e,
-    struct timeval *b)
+prusage(struct rusage *r0, struct rusage *r1, struct timespec *e,
+    struct timespec *b)
 {
     time_t t =
     (r1->ru_utime.tv_sec - r0->ru_utime.tv_sec) * 100 +
@@ -125,7 +125,7 @@ prusage(struct rusage *r0, struct rusage
     struct varent *vp = adrof(STRtime);
 
     int     ms =
-    (e->tv_sec - b->tv_sec) * 100 + (e->tv_usec - b->tv_usec) / 10000;
+    (e->tv_sec - b->tv_sec) * 100 + (e->tv_nsec - b->tv_nsec) / 10000000;
 
     cp = "%Uu %Ss %E %P %X+%Dk %I+%Oio %Fpf+%Ww";
 
Index: bin/ksh/c_sh.c
===================================================================
RCS file: /cvs/src/bin/ksh/c_sh.c,v
retrieving revision 1.59
diff -u -p -r1.59 c_sh.c
--- bin/ksh/c_sh.c      4 Mar 2016 15:11:06 -0000       1.59
+++ bin/ksh/c_sh.c      12 Jul 2017 04:18:27 -0000
@@ -18,7 +18,8 @@
 
 #include "sh.h"
 
-static void p_time(struct shf *, int, struct timeval *, int, char *, char *);
+static void p_tv(struct shf *, int, struct timeval *, int, char *, char *);
+static void p_ts(struct shf *, int, struct timespec *, int, char *, char *);
 
 /* :, false and true */
 int
@@ -670,7 +671,7 @@ c_unset(char **wp)
 }
 
 static void
-p_time(struct shf *shf, int posix, struct timeval *tv, int width, char *prefix,
+p_tv(struct shf *shf, int posix, struct timeval *tv, int width, char *prefix,
     char *suffix)
 {
        if (posix)
@@ -683,18 +684,34 @@ p_time(struct shf *shf, int posix, struc
                    tv->tv_usec / 10000, suffix);
 }
 
+static void
+p_ts(struct shf *shf, int posix, struct timespec *ts, int width, char *prefix,
+    char *suffix)
+{
+       if (posix)
+               shf_fprintf(shf, "%s%*lld.%02ld%s", prefix ? prefix : "",
+                   width, (long long)ts->tv_sec, ts->tv_nsec / 10000000,
+                   suffix);
+       else
+               shf_fprintf(shf, "%s%*lldm%02lld.%02lds%s", prefix ? prefix : 
"",
+                   width, (long long)ts->tv_sec / 60,
+                   (long long)ts->tv_sec % 60,
+                   ts->tv_nsec / 10000000, suffix);
+}
+
+
 int
 c_times(char **wp)
 {
        struct rusage usage;
 
        (void) getrusage(RUSAGE_SELF, &usage);
-       p_time(shl_stdout, 0, &usage.ru_utime, 0, NULL, " ");
-       p_time(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n");
+       p_tv(shl_stdout, 0, &usage.ru_utime, 0, NULL, " ");
+       p_tv(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n");
 
        (void) getrusage(RUSAGE_CHILDREN, &usage);
-       p_time(shl_stdout, 0, &usage.ru_utime, 0, NULL, " ");
-       p_time(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n");
+       p_tv(shl_stdout, 0, &usage.ru_utime, 0, NULL, " ");
+       p_tv(shl_stdout, 0, &usage.ru_stime, 0, NULL, "\n");
 
        return 0;
 }
@@ -710,11 +727,12 @@ timex(struct op *t, int f, volatile int 
 #define TF_POSIX       BIT(2)          /* report in posix format */
        int rv = 0;
        struct rusage ru0, ru1, cru0, cru1;
-       struct timeval usrtime, systime, tv0, tv1;
+       struct timeval usrtime, systime;
+       struct timespec ts0, ts1, ts2;
        int tf = 0;
        extern struct timeval j_usrtime, j_systime; /* computed by j_wait */
 
-       gettimeofday(&tv0, NULL);
+       clock_gettime(CLOCK_MONOTONIC, &ts0);
        getrusage(RUSAGE_SELF, &ru0);
        getrusage(RUSAGE_CHILDREN, &cru0);
        if (t->left) {
@@ -731,7 +749,7 @@ timex(struct op *t, int f, volatile int 
                rv = execute(t->left, f | XTIME, xerrok);
                if (t->left->type == TCOM)
                        tf |= t->left->str[0];
-               gettimeofday(&tv1, NULL);
+               clock_gettime(CLOCK_MONOTONIC, &ts1);
                getrusage(RUSAGE_SELF, &ru1);
                getrusage(RUSAGE_CHILDREN, &cru1);
        } else
@@ -749,20 +767,20 @@ timex(struct op *t, int f, volatile int 
        }
 
        if (!(tf & TF_NOREAL)) {
-               timersub(&tv1, &tv0, &tv1);
+               timespecsub(&ts1, &ts0, &ts2);
                if (tf & TF_POSIX)
-                       p_time(shl_out, 1, &tv1, 5, "real ", "\n");
+                       p_ts(shl_out, 1, &ts2, 5, "real ", "\n");
                else
-                       p_time(shl_out, 0, &tv1, 5, NULL, " real ");
+                       p_ts(shl_out, 0, &ts2, 5, NULL, " real ");
        }
        if (tf & TF_POSIX)
-               p_time(shl_out, 1, &usrtime, 5, "user ", "\n");
+               p_tv(shl_out, 1, &usrtime, 5, "user ", "\n");
        else
-               p_time(shl_out, 0, &usrtime, 5, NULL, " user ");
+               p_tv(shl_out, 0, &usrtime, 5, NULL, " user ");
        if (tf & TF_POSIX)
-               p_time(shl_out, 1, &systime, 5, "sys  ", "\n");
+               p_tv(shl_out, 1, &systime, 5, "sys  ", "\n");
        else
-               p_time(shl_out, 0, &systime, 5, NULL, " system\n");
+               p_tv(shl_out, 0, &systime, 5, NULL, " system\n");
        shf_flush(shl_out);
 
        return rv;
Index: usr.bin/time/time.c
===================================================================
RCS file: /cvs/src/usr.bin/time/time.c,v
retrieving revision 1.21
diff -u -p -r1.21 time.c
--- usr.bin/time/time.c 10 Oct 2015 14:49:23 -0000      1.21
+++ usr.bin/time/time.c 12 Jul 2017 04:19:40 -0000
@@ -52,7 +52,7 @@ main(int argc, char *argv[])
 {
        pid_t pid;
        int ch, status;
-       struct timeval before, after;
+       struct timespec before, after, during;
        struct rusage ru;
        int exitonsig = 0;
 
@@ -79,7 +79,7 @@ main(int argc, char *argv[])
        if (argc < 1)
                usage();
 
-       gettimeofday(&before, (struct timezone *)NULL);
+       clock_gettime(CLOCK_MONOTONIC, &before);
        switch(pid = vfork()) {
        case -1:                        /* error */
                perror("time");
@@ -97,24 +97,23 @@ main(int argc, char *argv[])
        (void)signal(SIGQUIT, SIG_IGN);
        while (wait3(&status, 0, &ru) != pid)
                ;
-       gettimeofday(&after, (struct timezone *)NULL);
+       clock_gettime(CLOCK_MONOTONIC, &after);
        if (WIFSIGNALED(status))
                exitonsig = WTERMSIG(status);
        if (!WIFEXITED(status))
                fprintf(stderr, "Command terminated abnormally.\n");
-       timersub(&after, &before, &after);
+       timespecsub(&after, &before, &during);
 
        if (portableflag) {
                fprintf(stderr, "real %9lld.%02ld\n",
-                       (long long)after.tv_sec, after.tv_usec/10000);
+                       (long long)during.tv_sec, after.tv_nsec/10000000);
                fprintf(stderr, "user %9lld.%02ld\n",
                        (long long)ru.ru_utime.tv_sec, 
ru.ru_utime.tv_usec/10000);
                fprintf(stderr, "sys  %9lld.%02ld\n",
                        (long long)ru.ru_stime.tv_sec, 
ru.ru_stime.tv_usec/10000);
        } else {
-
                fprintf(stderr, "%9lld.%02ld real ",
-                       (long long)after.tv_sec, after.tv_usec/10000);
+                       (long long)during.tv_sec, after.tv_nsec/10000000);
                fprintf(stderr, "%9lld.%02ld user ",
                        (long long)ru.ru_utime.tv_sec, 
ru.ru_utime.tv_usec/10000);
                fprintf(stderr, "%9lld.%02ld sys\n",

Reply via email to