Fix the max precision of the 'time' keyword

NEWS

@@ -8,6 +8,18 @@ Any uppercase BUG_* names are modernish shell bug IDs.
 - Fixed a fork bomb that could occur when the vi editor was sent SIGTSTP
   while running in a ksh script.
 
+- Appending a lone percent to the end of a format specifier no longer
+  causes a syntax error. The extra percent will be treated as a literal
+  '%', like in Bash and zsh.
+
+- The 'time' keyword now has proper support for millisecond precision.
+  Although this feature was previously documented, the 'time' keyword
+  only supported up to centisecond precision, which caused a command
+  like the one below to return '0.000' on certain operating systems:
+  $ TIMEFORMAT='%3R'; time sleep .003
+
+- The 'time' keyword now zero-pads seconds less than ten (like mksh).
+
 2020-07-10:
 
 - Fixed a bug that caused types created with 'typeset -T' to throw an error

src/cmd/ksh93/features/time

@@ -1,5 +1,5 @@
-hdr	utime
-lib	gettimeofday,setitimer
+hdr	utime,sys/resource
+lib	getrusage,gettimeofday,setitimer
 mem	timeval.tv_usec sys/time.h
 tst	lib_2_timeofday note{ 2 arg gettimeofday() }end link{
 	#include <sys/types.h>
@@ -23,6 +23,7 @@ tst	lib_1_timeofday note{ 1 arg gettimeofday() }end link{
 cat{
 	#undef _def_time
 	#include	<times.h>
+	#include	<sys/time.h>
 	#define _def_time	1
 	#undef timeofday
 	#if _lib_2_timeofday
@@ -32,4 +33,27 @@ cat{
 	#define timeofday(p)	gettimeofday(p)
 	#endif
 	#endif
+	/* BSD timeradd and timersub macros */
+	#if !defined(timeradd)
+	#define	timeradd(tvp, uvp, vvp)						\
+	do {								\
+		(vvp)->tv_sec = (tvp)->tv_sec + (uvp)->tv_sec;		\
+		(vvp)->tv_usec = (tvp)->tv_usec + (uvp)->tv_usec;	\
+		if ((vvp)->tv_usec >= 1000000) {			\
+			(vvp)->tv_sec++;				\
+			(vvp)->tv_usec -= 1000000;			\
+		}							\
+	} while(0)
+	#endif
+	#if !defined(timersub)
+	#define	timersub(tvp, uvp, vvp)						\
+	do {								\
+		(vvp)->tv_sec = (tvp)->tv_sec - (uvp)->tv_sec;		\
+		(vvp)->tv_usec = (tvp)->tv_usec - (uvp)->tv_usec;	\
+		if ((vvp)->tv_usec < 0) {				\
+			(vvp)->tv_sec--;				\
+			(vvp)->tv_usec += 1000000;			\
+		}							\
+	} while(0)
+	#endif
 }end

src/cmd/ksh93/sh/xec.c

@@ -95,6 +95,56 @@ struct funenv
     }
 #endif /* !SHOPT_DEVFD */
 
+#if _lib_getrusage
+/* getrusage tends to have higher precision */
+static void get_cpu_times(Shell_t *shp, struct timeval *tv_usr, struct timeval *tv_sys)
+{
+	NOT_USED(shp);
+	struct rusage usage_self, usage_child;
+
+	getrusage(RUSAGE_SELF, &usage_self);
+	getrusage(RUSAGE_CHILDREN, &usage_child);
+	timeradd(&usage_self.ru_utime, &usage_child.ru_utime, tv_usr);
+	timeradd(&usage_self.ru_stime, &usage_child.ru_stime, tv_sys);
+}
+#else
+#ifdef timeofday
+static void get_cpu_times(Shell_t *shp, struct timeval *tv_usr, struct timeval *tv_sys)
+{
+	struct tms cpu_times;
+	struct timeval tv1, tv2;
+	double dtime;
+
+	if(times(&cpu_times) == (clock_t)-1)
+		errormsg(SH_DICT, ERROR_exit(1), "times(3) failed: %s", strerror(errno));
+
+	dtime = (double)cpu_times.tms_utime / shp->gd->lim.clk_tck;
+	tv1.tv_sec = dtime / 60;
+	tv1.tv_usec = 1000000 * (dtime - tv1.tv_sec);
+	dtime = (double)cpu_times.tms_cutime / shp->gd->lim.clk_tck;
+	tv2.tv_sec = dtime / 60;
+	tv2.tv_usec = 1000000 * (dtime - tv2.tv_sec);
+	timeradd(&tv1, &tv2, tv_usr);
+
+	dtime = (double)cpu_times.tms_stime / shp->gd->lim.clk_tck;
+	tv1.tv_sec = dtime / 60;
+	tv1.tv_usec = 1000000 * (dtime - tv1.tv_sec);
+	dtime = (double)cpu_times.tms_cstime / shp->gd->lim.clk_tck;
+	tv2.tv_sec = dtime / 60;
+	tv2.tv_usec = 1000000 * (dtime - tv2.tv_sec);
+	timeradd(&tv1, &tv2, tv_sys);
+}
+#endif /* timeofday */
+#endif /* _lib_getrusage */
+
+#ifdef timeofday
+/* 'inline' is commented out because C89 doesn't have it */
+static /*inline*/ double timeval_to_double(struct timeval tv)
+{
+	return (double)tv.tv_sec + ((double)tv.tv_usec / 1000000.0);
+}
+#endif
+
 /*
  * The following two functions allow command substitution for non-builtins
  * to use a pipe and to wait for the pipe to close before restoring to a
@@ -182,11 +232,25 @@ static void iounpipe(Shell_t *shp)
 /*
  * print time <t> in h:m:s format with precision <p>
  */
-static void     l_time(Sfio_t *outfile,register clock_t t,int p)
+#ifdef timeofday
+static void l_time(Sfio_t *outfile, struct timeval *tv, int precision)
 {
-	register int  min, sec, frac;
+	int hr = tv->tv_sec / (60 * 60);
+	int min = (tv->tv_sec / 60) % 60;
+	int sec = tv->tv_sec % 60;
+	int frac = tv->tv_usec;
+
+	/* scale fraction from micro to milli, centi, or deci second according to precision */
+	int n;
+	for(n = 3 + (3 - precision); n > 0; --n)
+		frac /= 10;
+#else
+/* fallback */
+static void l_time(Sfio_t *outfile,register clock_t t,int precision)
+{
+	register int min, sec, frac;
 	register int hr;
-	if(p)
+	if(precision)
 	{
 		frac = t%shgd->lim.clk_tck;
 		frac = (frac*100)/shgd->lim.clk_tck;
@@ -195,49 +259,113 @@ static void     l_time(Sfio_t *outfile,register clock_t t,int p)
 	sec = t%60;
 	t /= 60;
 	min = t%60;
-	if(hr=t/60)
+	hr = t/60;
+#endif
+	if(hr)
 		sfprintf(outfile,"%dh",hr);
-	if(p)
-		sfprintf(outfile,"%dm%d%c%0*ds",min,sec,GETDECIMAL(0),p,frac);
+	if(precision)
+		sfprintf(outfile,"%dm%02d%c%0*ds",min,sec,GETDECIMAL(0),precision,frac);
 	else
-		sfprintf(outfile,"%dm%ds",min,sec);
+		sfprintf(outfile,"%dm%02ds",min,sec);
 }
 
-static int p_time(Shell_t *shp, Sfio_t *out, const char *format, clock_t *tm)
+#define TM_REAL_IDX 0
+#define TM_USR_IDX 1
+#define TM_SYS_IDX 2
+
+#ifdef timeofday
+static void p_time(Shell_t *shp, Sfio_t *out, const char *format, struct timeval tm[3])
+#else
+static void p_time(Shell_t *shp, Sfio_t *out, const char *format, clock_t *tm)
+#endif
 {
-	int		c,p,l,n,offset = staktell();
+	int		c,n,offset = staktell();
 	const char	*first;
-	double		d;
 	Stk_t		*stkp = shp->stk;
-	for(first=format ; c= *format; format++)
+#ifdef timeofday
+	struct timeval tv_cpu_sum;
+	struct timeval *tvp;
+#else
+	double d;
+#endif
+	for(first=format; *format; format++)
 	{
+		c = *format;
 		if(c!='%')
 			continue;
+		unsigned char l_modifier = 0;
+		int precision = 3;
+#ifndef timeofday
+		n = 0;
+#endif
+
 		sfwrite(stkp, first, format-first);
-		n = l = 0;
-		p = 3;
-		if((c= *++format) == '%')
+		c = *++format;
+		if(c=='\0')
+		{
+			/* If a lone percent is the last character of the format pretend
+			   the user had written `%%` for a literal percent */
+			sfwrite(stkp, "%", 1);
+			first = format + 1;
+			break;
+		}
+		else if(c=='%')
 		{
 			first = format;
 			continue;
 		}
 		if(c>='0' && c <='9')
 		{
-			p = (c>'3')?3:(c-'0');
+			precision = (c>'3')?3:(c-'0');
 			c = *++format;
 		}
-		else if(c=='P')
+		if(c=='P')
 		{
+#ifdef timeofday
+			struct timeval tv_real = tm[TM_REAL_IDX];
+			struct timeval tv_cpu;
+			timeradd(&tm[TM_USR_IDX], &tm[TM_SYS_IDX], &tv_cpu);
+
+			double d = timeval_to_double(tv_real);
+			if(d)
+				d = 100.0 * timeval_to_double(tv_cpu) / d;
+			sfprintf(stkp, "%.*f", precision, d);
+			first = format + 1;
+			continue;
+#else
 			if(d=tm[0])
 				d = 100.*(((double)(tm[1]+tm[2]))/d);
-			p = 2;
+			precision = 2;
 			goto skip;
+#endif
 		}
 		if(c=='l')
 		{
-			l = 1;
+			l_modifier = 1;
 			c = *++format;
 		}
+#ifdef timeofday
+		if(c=='R')
+			tvp = &tm[TM_REAL_IDX];
+		else if(c=='U')
+			tvp = &tm[TM_USR_IDX];
+		else if(c=='S')
+			tvp = &tm[TM_SYS_IDX];
+		else
+		{
+			errormsg(SH_DICT,ERROR_exit(0),e_badtformat,c);
+			continue;
+		}
+		if(l_modifier)
+			l_time(stkp, tvp, precision);
+		else
+		{
+			/* scale fraction from micro to milli, centi, or deci second according to precision */
+			int n, frac = tvp->tv_usec;
+			for(n = 3 + (3 - precision); n > 0; --n) frac /= 10;
+			sfprintf(stkp, "%d%c%0*d", tvp->tv_sec, GETDECIMAL(0), precision, frac);
+		}
+#else
 		if(c=='U')
 			n = 1;
 		else if(c=='S')
@@ -246,14 +374,15 @@ static int p_time(Shell_t *shp, Sfio_t *out, const char *format, clock_t *tm)
 		{
 			stkseek(stkp,offset);
 			errormsg(SH_DICT,ERROR_exit(0),e_badtformat,c);
-			return(0);
+			return;
 		}
 		d = (double)tm[n]/shp->gd->lim.clk_tck;
 	skip:
-		if(l)
-			l_time(stkp, tm[n], p);
+		if(l_modifier)
+			l_time(stkp, tm[n], precision);
 		else
-			sfprintf(stkp,"%.*f",p, d);
+			sfprintf(stkp,"%.*f",precision, d);
+#endif
 		first = format+1;
 	}
 	if(format>first)
@@ -262,7 +391,6 @@ static int p_time(Shell_t *shp, Sfio_t *out, const char *format, clock_t *tm)
 	n = stktell(stkp)-offset;
 	sfwrite(out,stkptr(stkp,offset),n);
 	stkseek(stkp,offset);
-	return(n);
 }
 
 #if SHOPT_OPTIMIZE
@@ -2577,14 +2705,14 @@ int sh_exec(register const Shnode_t *t, int flags)
 		    case TTIME:
 		    {
 			/* time the command */
-			struct tms before,after;
 			const char *format = e_timeformat;
-			clock_t at, tm[3];
+			struct timeval ta, tb;
 #ifdef timeofday
-			struct timeval tb,ta;
+			struct timeval before_usr, before_sys, after_usr, after_sys, tm[3];
 #else
-			clock_t bt;
-#endif	/* timeofday */
+			struct tms before,after;
+			clock_t at, bt, tm[3];
+#endif
 #if SHOPT_COSHELL
 			if(shp->inpool)
 			{
@@ -2601,16 +2729,18 @@ int sh_exec(register const Shnode_t *t, int flags)
 			}
 			if(t->par.partre)
 			{
-				long timer_on;
 				if(shp->subshell && shp->comsub==1)
 					sh_subfork();
-				timer_on = sh_isstate(SH_TIMING);
+				long timer_on = sh_isstate(SH_TIMING);
 #ifdef timeofday
+				/* must be run after forking a subshell */
 				timeofday(&tb);
-				times(&before);
+				get_cpu_times(shp, &before_usr, &before_sys);
 #else
 				bt = times(&before);
-#endif	/* timeofday */
+				if(bt == (clock_t)-1)
+					errormsg(SH_DICT, ERROR_exit(1), "times(3) failed: %s", strerror(errno));
+#endif
 				job.waitall = 1;
 				sh_onstate(SH_TIMING);
 				sh_exec(t->par.partre,OPTIMIZE);
@@ -2620,21 +2750,28 @@ int sh_exec(register const Shnode_t *t, int flags)
 			}
 			else
 			{
-#ifndef timeofday
+#ifdef timeofday
+				before_usr.tv_sec = before_usr.tv_usec = 0;
+				before_sys.tv_sec = before_sys.tv_usec = 0;
+#else
 				bt = 0;
-#endif	/* timeofday */
 				before.tms_utime = before.tms_cutime = 0;
 				before.tms_stime = before.tms_cstime = 0;
+#endif
 			}
-#ifdef timeofday
-			times(&after);
-			timeofday(&ta);
-			at = shp->gd->lim.clk_tck*(ta.tv_sec-tb.tv_sec);
-			at +=  ((shp->gd->lim.clk_tck*(((1000000L/2)/shp->gd->lim.clk_tck)+(ta.tv_usec-tb.tv_usec)))/1000000L);
-#else
+#ifndef timeofday
 			at = times(&after) - bt;
-#endif	/* timeofday */
+			if(at == (clock_t)-1)
+				errormsg(SH_DICT, ERROR_exit(1), "times(3) failed: %s", strerror(errno));
 			tm[0] = at;
+#else
+			get_cpu_times(shp, &after_usr, &after_sys);
+			timeofday(&ta);
+			timersub(&ta, &tb, &tm[TM_REAL_IDX]); /* calculate elapsed real-time */
+			timersub(&after_usr, &before_usr, &tm[TM_USR_IDX]);
+			timersub(&after_sys, &before_sys, &tm[TM_SYS_IDX]);
+#endif
+
 			if(t->par.partre)
 			{
 				Namval_t *np = nv_open("TIMEFORMAT",shp->var_tree,NV_NOADD);
@@ -2648,10 +2785,12 @@ int sh_exec(register const Shnode_t *t, int flags)
 			}
 			else
 				format = strchr(format+1,'\n')+1;
+#ifndef timeofday
 			tm[1] = after.tms_utime - before.tms_utime;
 			tm[1] += after.tms_cutime - before.tms_cutime;
 			tm[2] = after.tms_stime - before.tms_stime;
 			tm[2] += after.tms_cstime - before.tms_cstime;
+#endif
 			if(format && *format)
 				p_time(shp,sfstderr,sh_translate(format),tm);
 			break;