On Tue, Aug 20, 2024 at 09:14:40AM -0400, Mike Pattrick wrote:
> The Clang analyzer will alert on the use of uninitilized variable local
> despite the fact that this should be set by a syscall.
> 
> To suprress the warning, this variable is now initialized.

Hi Mike,

I agree we should update code that is incorrect.  And annotate or make it
easier easier for tooling to understand the correctness of is also good
IMHO.  But I don't think we should update code just to suppress warnings.

If local.nl_family is supposed to be set by getsockname() (unless it
returns an error) then I don't think this change is a good one.
Rather, I think the tooling should be enhanced (ok, I know that is hard).

I won't push this if others think otherwise.
But in my opinion this one is over the line.

> 
> Signed-off-by: Mike Pattrick <[email protected]>
> ---
>  lib/netlink-socket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/netlink-socket.c b/lib/netlink-socket.c
> index 5cb1fc89a..737e49cfc 100644
> --- a/lib/netlink-socket.c
> +++ b/lib/netlink-socket.c
> @@ -233,6 +233,7 @@ nl_sock_create(int protocol, struct nl_sock **sockp)
>  
>      /* Obtain pid assigned by kernel. */
>      local_size = sizeof local;
> +    local.nl_family = AF_UNSPEC;
>      if (getsockname(sock->fd, (struct sockaddr *) &local, &local_size) < 0) {
>          VLOG_ERR("getsockname: %s", ovs_strerror(errno));
>          goto error;
> -- 
> 2.43.5
> 
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to