On Thu, Nov 30, 2017 at 06:32:55AM -0800, Yifeng Sun wrote:
> netdev_get_etheraddr claims to clear 'mac' on error, but it fails to do so.
> When looking further into both netdev_windows_get_etheraddr() and
> netdev_linux_get_etheraddr(), 'mac' is also not cleared. This will lead to
> usage of uninitialised ofputil_phy_port.hw_addr.
> 
> Signed-off-by: Yifeng Sun <[email protected]>
> ---
>  lib/netdev.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/netdev.c b/lib/netdev.c
> index c52680659e3f..f62c41b7601c 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -897,7 +897,13 @@ netdev_set_etheraddr(struct netdev *netdev, const struct 
> eth_addr mac)
>  int
>  netdev_get_etheraddr(const struct netdev *netdev, struct eth_addr *mac)
>  {
> -    return netdev->netdev_class->get_etheraddr(netdev, mac);
> +    int ret;
> +
> +    ret = netdev->netdev_class->get_etheraddr(netdev, mac);
> +    if (!ret) {
> +        memset(mac, 0, sizeof *mac);
> +    }
> +    return ret;
>  }

Thank you for working to improve this code.

I think that this is backward, because a return value of 0 indicates
success but this interprets it as failure.  Can you take a second look?

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to