Would you please check libntp/systime.c
Eric S. Raymond
esr at thyrsus.com
Tue Jun 7 22:13:03 UTC 2016
Hal Murray <hmurray at megapathdsl.net>:
> The initial symptom is a warning from clang 3.8.0 on a Raspberry Pi.
> ../../libntp/systime.c:460:37: warning: variable 'tvlast' is uninitialized
> used here [-Wuninitialized]
> Why didn't any of the other tools notice this? The code isn't particularly
I don't know. It does seem like the sort of error a static analyzer should spot.
> A diff with current ntp classic is 329 lines. Most of the changes look
> reasonablle. It;s POSIXifying writing time steps for the accounting system.
> But the warning looks like a bug to me, and I can't figure out some of the
> other changes.
> < /* get the current time as l_fp (without fuzz) and as struct timespec
> > /* get the current time as l_fp (without fuzz) and as struct timeval */
> > tvlast.tv_sec = timets.tv_sec;
> > tvlast.tv_usec = (timets.tv_nsec + 500) / 1000;
> Those lines got dropped. That's where tvlast got setup.
> < tvlast = timetv;
> < return true;
> > return TRUE;
> The assignment to tvlast doesn't make sense. It's a local variable so goes
> away with the return.
Right, I've fixed it. The reason the accidental removal of setting
tvlast didn't cause any damage I could spot is that it was only used
for logging stepping actions. I think there used to be (my) code
after the assignment that used it.
That logging code is really dodgy. It's unconditionally disabled in Classic.
I fixed it, because I could, and then later broke it by deleting the tvlast
setting. I should probably just rip it out.
I keep thinking that whenever I look at it, but it's kind of a cute use
for those logging channels and I haven't been able to make myself pull
the trigger yet.
<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>
More information about the devel