On Tue, Jul 05, 2022 at 11:34:21AM +0000, Job Snijders wrote:
> On Tue, Jul 05, 2022 at 11:08:13AM +0200, Claudio Jeker wrote:
> > On Mon, Jul 04, 2022 at 05:10:05PM -0500, Scott Cheloha wrote:
> > > On Mon, Jul 04, 2022 at 11:15:24PM +0200, Claudio Jeker wrote:
> > > > On Mon, Jul 04, 2022 at 01:28:12PM -0500, Scott Cheloha wrote:
> > > > > Hi,
> > > > >
> > > > > Couple things:
> > > > >
> > > > > [...]
> > > >
> > > > I don't like the introduction of all these local variables that are just
> > > > hard to follow and need extra code pathes. Happy to rename roff to
> > > > offset,
> > > > start_offset or something similar. Also moving the localtime call into
> > > > fmtfmt() is fine.
> > >
> > > You need an "elapsed" variable to avoid overwriting "now" in the
> > > -i flag case to avoid calling clock_gettime(2) twice.
> > >
> > > We can get rid of "utc_start" and just reuse "now" for the initial
> > > value of CLOCK_REALTIME.
> > >
> > > How is this?
> >
> > How about this instead?
>
> Looks like an improvement
>
> The suggestion to change 'ms' to 'us' might be a good one to roll into
> this changeset too.
Ah right, we print us not ms.
> nitpick: the changeset doesn't apply cleanly:
Forgot to update that tree :)
Updated diff below
--
:wq Claudio
Index: ts.c
===================================================================
RCS file: /cvs/src/usr.bin/ts/ts.c,v
retrieving revision 1.6
diff -u -p -r1.6 ts.c
--- ts.c 4 Jul 2022 17:29:03 -0000 1.6
+++ ts.c 5 Jul 2022 09:51:38 -0000
@@ -32,7 +32,7 @@ static char *buf;
static char *outbuf;
static size_t bufsize;
-static void fmtfmt(struct tm *, long);
+static void fmtfmt(const struct timespec *);
static void __dead usage(void);
int
@@ -40,8 +40,7 @@ main(int argc, char *argv[])
{
int iflag, mflag, sflag;
int ch, prev;
- struct timespec roff, start, now;
- struct tm *tm;
+ struct timespec start, now, utc_offset, ts;
clockid_t clock = CLOCK_REALTIME;
if (pledge("stdio", NULL) == -1)
@@ -93,22 +92,22 @@ main(int argc, char *argv[])
if (setenv("TZ", "UTC", 1) == -1)
err(1, "setenv UTC");
- clock_gettime(CLOCK_REALTIME, &roff);
clock_gettime(clock, &start);
- timespecsub(&roff, &start, &roff);
+ clock_gettime(CLOCK_REALTIME, &utc_offset);
+ timespecsub(&utc_offset, &start, &utc_offset);
for (prev = '\n'; (ch = getchar()) != EOF; prev = ch) {
if (prev == '\n') {
clock_gettime(clock, &now);
if (iflag || sflag)
- timespecsub(&now, &start, &now);
+ timespecsub(&now, &start, &ts);
else if (mflag)
- timespecadd(&now, &roff, &now);
+ timespecadd(&now, &utc_offset, &ts);
+ else
+ ts = now;
+ fmtfmt(&ts);
if (iflag)
- clock_gettime(clock, &start);
- if ((tm = localtime(&now.tv_sec)) == NULL)
- err(1, "localtime");
- fmtfmt(tm, now.tv_nsec);
+ start = now;
}
if (putchar(ch) == EOF)
break;
@@ -132,11 +131,15 @@ usage(void)
* so you can format while you format
*/
static void
-fmtfmt(struct tm *tm, long tv_nsec)
+fmtfmt(const struct timespec *ts)
{
- char *f, ms[7];
+ struct tm *tm;
+ char *f, us[7];
+
+ if ((tm = localtime(&ts->tv_sec)) == NULL)
+ err(1, "localtime");
- snprintf(ms, sizeof(ms), "%06ld", tv_nsec / 1000);
+ snprintf(us, sizeof(us), "%06ld", ts->tv_nsec / 1000);
strlcpy(buf, format, bufsize);
f = buf;
@@ -157,7 +160,7 @@ fmtfmt(struct tm *tm, long tv_nsec)
f += 2;
l = strlen(f);
memmove(f + 6, f, l + 1);
- memcpy(f, ms, 6);
+ memcpy(f, us, 6);
f += 6;
}
} while (*f != '\0');
