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?