On 03/10/2016 12:21 PM, Andrea Bolognani wrote:
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?
size_t i; Coverity says: (60) Event negative_returns: Using unsigned variable "last_processed_hostdev_vf" in a loop exit condition. John