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@xxxxxxxxxx> --- 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
