On Tue, Jul 05, 2022 at 11:53:26AM +0200, Claudio Jeker wrote:
> 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
This is fine by me, you took most of what I wanted, and even
the "ms" -> "us" name change :)
One nit below, otherwise: ok cheloha@
> 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);
You don't need to initialize utc_offset except in the -m flag case.
