Would you please check libntp/systime.c

[Eric S. Raymond]

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 
> when
> used here [-Wuninitialized]
> 
> Why didn't any of the other tools notice this?  The code isn't particularly 
> complicated.

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 */
> 415a432,433
> >       tvlast.tv_sec = timets.tv_sec;
> >       tvlast.tv_usec = (timets.tv_nsec + 500) / 1000;
> Those lines got dropped.  That's where tvlast got setup.
> 
> 501,502c596
> <       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>