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