[libvirt] [PATCH 11/24] hostdev: Remove redundant check

[Andrea Bolognani]

On Thu, 2016-03-10 at 12:35 -0500, John Ferlan wrote:
> > > If for some reason we "continue" (or not) and eventually "goto resetvfnetconfig;"
> > > before ever setting last_processed_hostdev_vf, then we get to the goto.,..
> > >  
> > > >    resetvfnetconfig:
> > > > -    for (i = 0;
> > > > -         last_processed_hostdev_vf != -1 && i <= last_processed_hostdev_vf; i++)
> > > and last_processed_hostdev_vf still == -1
> > >  
> > > So that check needs to be there - perhaps just add an:
> > >  
> > >      if (last_processed_hostdev_vf > -1) {
> > >      }
> > I fail to see how this is a problem: if last_processed_hostdev_vf
> > has never been assigned a value after being declared, then the
> > rewritten loop would be equivalent to
> > 
> >   for (i = 0; i <= -1; i++) { ... }
> > 
> > which means the loop body will never be executed, just as
> > expected. Am I missing something?
> 
> size_t i;
> 
> Coverity says:
> 
> (60) Event negative_returns:  Using unsigned variable
> "last_processed_hostdev_vf" in a loop exit condition.

I admit I had to do some research, but I get it now.

Not having any experience with the tool, its output looks very
confusing to me[1]; thankfully you are fluent in Coverity and
pointed me in the right direction :)

Will change it to move the check on last_processed_hostdev_vf
outside the for loop as you suggested, it might not be as nice
as getting rid of it altogether but it's still much more
readable than the current code.

Cheers.


[1] Why does it claim that 'last_processed_hostdev_vf' is an
    unsigned variable when it's not? And why would using an
    unsigned variable in loop exit condition automatically be
    a problem?
-- 
Andrea Bolognani
Software Engineer - Virtualization Team