[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On Thu, 2016-03-10 at 12:01 -0500, John Ferlan wrote:
> On 03/07/2016 12:24 PM, Andrea Bolognani wrote:
> > 
> > If 'last_processed_hostdev_vf != -1' is false then, since the
> > loop counter 'i' starts at 0, 'i <= last_processed_hostdev_vf'
> > can't possibly be true and the loop body will never be executed.
> > 
> > Hence, the first check is completely redundant and can be safely
> > removed.
> > ---
> >  src/util/virhostdev.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> Premise understood; however, Coverity has an issue...

Well, that's a first :P

> Way back here:
> 
> 507       virPCIDeviceListPtr pcidevs = NULL;
> 
> (1) Event var_tested_neg:     Assigning: "last_processed_hostdev_vf" = a negative value.
> Also see events:  [negative_returns]
> 
> 508       int last_processed_hostdev_vf = -1;
> 
> 
> Eventually we enter this loop:
> 
>     for (i = 0; i < nhostdevs; i++) {
>          virDomainHostdevDefPtr hostdev = hostdevs[i];
>          if (!virHostdevIsPCINetDevice(hostdev))
>              continue;
>          if (virHostdevNetConfigReplace(hostdev, uuid,
>                                         mgr->stateDir) < 0) {
>              goto resetvfnetconfig;
>          }
>          last_processed_hostdev_vf = i;
>     }
> 
> 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?

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]