On 5/23/24 21:11, Mike Pattrick wrote:
> Clang's static analyzer noted that the output from
> netdev_linux_get_speed_locked can be checked even if this function
> doesn't set any values.
> 
> Now we always set those values to a sane default in all cases.
> 
> Fixes: 6240c0b4c80e ("netdev: Add netdev_get_speed() to netdev API.")

This doesn't look like the right tag.  The issue, AFAICT, was introduced
when QoS functions started to use netdev_linux_get_speed_locked() without
checking the return value.  That happened much later.  Or am I missing
something?

The alternative would be to fix those functions instead, but this approach
is also fine.

> Signed-off-by: Mike Pattrick <[email protected]>
> ---
>  lib/netdev-linux.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index 8b855bdc4..83c19618c 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2726,6 +2726,8 @@ netdev_linux_get_speed_locked(struct netdev_linux 
> *netdev,
>                                uint32_t *current, uint32_t *max)
>  {
>      if (netdev_linux_netnsid_is_remote(netdev)) {
> +        *current = 0;
> +        *max = 0;
>          return EOPNOTSUPP;
>      }
>  
> @@ -2735,6 +2737,9 @@ netdev_linux_get_speed_locked(struct netdev_linux 
> *netdev,
>                     ? 0 : netdev->current_speed;
>          *max = MIN(UINT32_MAX,
>                     netdev_features_to_bps(netdev->supported, 0) / 
> 1000000ULL);
> +    } else {
> +        *current = 0;
> +        *max = 0;

Please, squash these into one line as netdev_get_speed() does.  Same for the 
other
instance.

>      }
>      return netdev->get_features_error;
>  }

Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to