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

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?