On Thu, Jul 25, 2019 at 09:37:20AM -0600, Todd C. Miller wrote:
> On Thu, 25 Jul 2019 09:53:42 -0500, Scott Cheloha wrote:
>
> > Woohoo, slow morning, nearly missed that bug. Your conditional
> > is incorrect because it rejects timevals of whole seconds, e.g.
> > tv_sec == 1 and tv_usec == 0.
>
> Whoops, too early for me I guess. However, as I think about this
> further, can't this be simplified even more?
>
> Given:
>
> if (itp->it_value.tv_sec >= 0 && timerisset(&itp->it_value))
>
> that expands to:
>
> if (itp->it_value.tv_sec >= 0 &&
> (itp->it_value.tv_sec || itp->it_value.tv_usec))
>
> However, if the "itp->it_value.tv_sec >= 0" is true, that means
> that "itp->it_value.tv_sec" must also be true. So logically it
> is the equivalent:
>
> if (itp->it_value.tv_sec >= 0 && (1 || itp->it_value.tv_usec))
>
> Which is can be simplied to just:
>
> if (itp->it_value.tv_sec >= 0)
>
> So the timerisset() call appears to be superfluous. I see no case
> where "itp->it_value.tv_sec >= 0" is true that timerisset(&itp->it_value)
> is not also true.
>
> Am I missing something?
This is indeed the expansion:
> if (itp->it_value.tv_sec >= 0 &&
> (itp->it_value.tv_sec || itp->it_value.tv_usec))
But then you say:
> However, if the "itp->it_value.tv_sec >= 0" is true, that means
> that "itp->it_value.tv_sec" must also be true.
which is not correct, as itp->it_value.tv_sec could be zero.
What we really want to check is:
if (itp->it_value.tv_sec > 0 ||
(itp->it_value.tv_sec == 0 && itp->it_value.tv_usec > 0))
which is nearly equivalent to the expansion above. The expansion
above ignores the sign of itp->it_value.tv_usec, so, not exactly the
same.
