[libvirt] [PATCH 2/3] network: Use local variables in networkUpdatePortBandwidth

[Michal Privoznik]

On 11/18/19 2:27 PM, Daniel Henrique Barboza wrote:
> 
> 
> On 11/14/19 6:58 PM, John Ferlan wrote:
>> We go through the trouble of checking {old|new}Bandwidth[->in] and
>> storing the result in local @old_floor and @new_floor, but then
>> we don't use them. Instead we make derefs to the longer name. This
>> caused Coverity to note dereferencing newBandwidth->in without first
>> checking @newBandwidth like was done for new_floor could cause a
>> NULL dereference.
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>   src/network/bridge_driver.c | 17 ++++++++---------
>>   1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>> index 7ee5d7ee53..68bb916501 100644
>> --- a/src/network/bridge_driver.c
>> +++ b/src/network/bridge_driver.c
>> @@ -5380,19 +5380,18 @@ networkUpdatePortBandwidth(virNetworkObjPtr obj,
>>       /* Okay, there are three possible scenarios: */
>> -    if (oldBandwidth && oldBandwidth->in && oldBandwidth->in->floor &&
>> -        newBandwidth->in && newBandwidth->in->floor) {
>> +    if (old_floor > 0 && new_floor > 0) {
> 
> 
> Nit: both old_floor and new_floor are unsigned, thus comparing them to > 0
> or doing (old_floor && new_floor) like it was being done before is the same
> thing. Same deal with the 'if (new_floor > 0)' down below.
> 
> I don't mind the extra clarity though.

I believe it was John who persuaded us to use explicit integer 
comparison for integer variables. The idea is that it's clear from the 
check itself if we are comparing integers or pointers. And I agree with 
him - in my new code I always use 'if (x > 0)' or 'if (x != 0)' instead 
of 'if (x)' or 'if (!x)'.

Michal