On Tue, Jul 05, 2022 at 07:04:49AM -0500, Scott Cheloha wrote:
> 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.
I did not do this because I think it is an micro-optimisation that is not
really worth it.
--
:wq Claudio
