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

Daniel Henrique Barboza danielhb413 at gmail.com
Mon Nov 18 13:27:00 UTC 2019



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.


Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>


>           /* Either we just need to update @floor .. */
>   
>           if (virNetDevBandwidthUpdateRate(def->bridge,
>                                            *class_id,
>                                            def->bandwidth,
> -                                         newBandwidth->in->floor) < 0)
> +                                         new_floor) < 0)
>               return -1;
>   
>           tmp_floor_sum = virNetworkObjGetFloorSum(obj);
> -        tmp_floor_sum -= oldBandwidth->in->floor;
> -        tmp_floor_sum += newBandwidth->in->floor;
> +        tmp_floor_sum -= old_floor;
> +        tmp_floor_sum += new_floor;
>           virNetworkObjSetFloorSum(obj, tmp_floor_sum);
>           new_rate -= tmp_floor_sum;
>   
> @@ -5401,17 +5400,17 @@ networkUpdatePortBandwidth(virNetworkObjPtr obj,
>               virNetworkObjSaveStatus(driver->stateDir,
>                                       obj, network_driver->xmlopt) < 0) {
>               /* Ouch, rollback */
> -            tmp_floor_sum -= newBandwidth->in->floor;
> -            tmp_floor_sum += oldBandwidth->in->floor;
> +            tmp_floor_sum -= new_floor;
> +            tmp_floor_sum += old_floor;
>               virNetworkObjSetFloorSum(obj, tmp_floor_sum);
>   
>               ignore_value(virNetDevBandwidthUpdateRate(def->bridge,
>                                                         *class_id,
>                                                         def->bandwidth,
> -                                                      oldBandwidth->in->floor));
> +                                                      old_floor));
>               return -1;
>           }
> -    } else if (newBandwidth->in && newBandwidth->in->floor) {
> +    } else if (new_floor > 0) {
>           /* .. or we need to plug in new .. */
>   
>           if (networkPlugBandwidthImpl(obj, mac, newBandwidth,
> 




More information about the libvir-list mailing list