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

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(a)redhat.com&gt; > --- >   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.