On Mon, Nov 22, 2021 at 01:40:34PM +0000, Klemens Nanni wrote:
> On Mon, Nov 22, 2021 at 09:30:13AM +0100, Claudio Jeker wrote:
> > > Index: sbin/ifconfig/ifconfig.c
> > > ===================================================================
> > > RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
> > > retrieving revision 1.450
> > > diff -u -p -r1.450 ifconfig.c
> > > --- sbin/ifconfig/ifconfig.c 17 Nov 2021 18:00:24 -0000 1.450
> > > +++ sbin/ifconfig/ifconfig.c 22 Nov 2021 00:25:04 -0000
> > > @@ -5362,12 +5362,13 @@ pppoe_status(void)
> > > printf(" PADR retries: %d", state.padr_retry_no);
> > >
> > > if (state.state == PPPOE_STATE_SESSION) {
> > > - struct timeval temp_time;
> > > + struct timespec temp_time;
> > > time_t diff_time, day = 0;
> > > unsigned int hour = 0, min = 0, sec = 0;
> > >
> > > if (state.session_time.tv_sec != 0) {
> > > - gettimeofday(&temp_time, NULL);
> > > + if (clock_gettime(CLOCK_BOOTTIME, &temp_time) == -1)
> > > + goto notime;
> > > diff_time = temp_time.tv_sec -
> > > state.session_time.tv_sec;
> > >
> > > @@ -5387,6 +5388,7 @@ pppoe_status(void)
> > > printf("%lldd ", (long long)day);
> > > printf("%02u:%02u:%02u", hour, min, sec);
> > > }
> > > +notime:
> > > putchar('\n');
> > > }
> > >
> >
> > The way you call clock_gettime() it can't fail. Apart from that this is
> > the right way of fixing this. OK claudio@
>
> Yes, I inferred that from clock_gettime(9)' ERRORS as well, but all
> other CLOCK_BOOTTIME users in base do handle the error case, so I went
> along.
>
> CLOCK_MONOTONIC users in base however consistently ignore the error case
> which made me think there is some pattern I don't yet understand fully.
I think this comes from the fact that CLOCK_BOOTTIME was not available all
the time while CLOCK_MONOTONIC is more or less supported everywhere.
I doubt protability concerns matter since ifconfig(8) is highly OpenBSD
specific. Also CLOCK_BOOTTIME is the same as CLOCK_MONOTONIC on
OpenBSD (both return nanouptime()).
Anyway, commit the way you prefer. You could also put the
clock_gettime(CLOCK_BOOTTIME, &temp_time) == -1 check in the first if
which also removes the goto:
if (state.session_time.tv_sec != 0 &&
clock_gettime(CLOCK_BOOTTIME, &temp_time) == 0) {
...
}
--
:wq Claudio
