timestamp_before_wait might be used uninitialized in this function

[Rob Landley]

On Monday 19 January 2009 20:57:35 Denys Vlasenko wrote:
> On Wednesday 14 January 2009 14:21, walter harms wrote:
> > >>   unsigned timestamp_before_wait=0;
> > >>
> > >> will do the same ?
> > >
> > > Yes. but
> > >
> > > unsigned timestamp_before_wait = timestamp_before_wait;
> > >
> > > is better wrt code size. :)
> > >
> > > fixed. thanks.
> >
> > aehm, that will only fool the compiler to thing it is used.

It shuts up the warning without generating extra code.  Sounds like what we 
want.

> > timestamp_before_wait=0 will make sure that it works even if the 'if
> > (tv.tv_sec > 0)' fails. and it will fail because:
> >
> >         timeout = 0;
> >         already_waited_sec = 0;
> > ...
> >         tv.tv_sec = timeout - already_waited_sec;

0-0 == 0 so tv.tv_sec gets set to a known value of 0 first time through the 
loop.  Ok.

> > 	if (tv.tv_sec > 0)
> > 		timestamp_before_wait = (unsigned)monotonic_sec();

Since 0>0 is false, the if() doesn't trigger: the first time through the loop.

However, we know what happens the first time thorugh the loop.  Right after 
that we go into the test for (retval==0), which has to be true because the 
only thing that could have set it to nonzero was in the if (tv.tv_sec > 0) bit 
that didn't trigger.  So we reach the switch statement and we know that state 
was initialized to INIT_SELECTING, which sets timeout (either to 
discover_timeout or tryagain_timeout) and does a continue which jumps back to 
the loop at the top.  And _this_ time through, timeout is going to be >0 so 
tv.tv_sec is set and the check for it triggers and sets timestamp_before_wait.

> > ...
> > 	already_waited_sec += (unsigned)monotonic_sec() - timestamp_before_wait;

So no, we can't reach that line without timestamp_before_wait having been set.  
We'd either exit the function first or set it.

> It's the choice between smaller code and more clearly written code.

"small", "simple", and "correct" are the conflicting goals of the busybox 
project.  Always have been. :)

I personally sort 'em simple, correct, then small.  But YMMV.

> In this particular case, do you see a way for timestamp_before_wait
> to be used w/o initialization?

I don't see how it can happen.

Rob