| Message ID | 20240820135549.1726748-5-mkp@redhat.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | Address clang analyze warnings. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
On 8/20/24 15:55, Mike Pattrick wrote: > The Clang analyzer will alert on the use of uninitialized variable local > despite the fact that this should be set by a syscall. > > To suppress the warning, this variable is now initialized. > > Signed-off-by: Mike Pattrick <mkp@redhat.com> > --- > 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; It feels strange to do that. Just memset-ing the whole structure to 0 might be a better choice. > if (getsockname(sock->fd, (struct sockaddr *) &local, &local_size) < 0) { > VLOG_ERR("getsockname: %s", ovs_strerror(errno)); > goto error;
On 28 Aug 2024, at 2:40, Ilya Maximets wrote: > On 8/20/24 15:55, Mike Pattrick wrote: >> The Clang analyzer will alert on the use of uninitialized variable local >> despite the fact that this should be set by a syscall. >> >> To suppress the warning, this variable is now initialized. >> >> Signed-off-by: Mike Pattrick <mkp@redhat.com> >> --- >> 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; > > It feels strange to do that. Just memset-ing the whole structure to 0 might > be a better choice. As Simon mentioned in the v1 patch, this is more a problem of clang than our code, and wondering if we want to fix the code around it. I just waved a couple of similar issues in Coverity. However, if we do want to fix this, I would prefer the memset() as Ilya suggested. I have not looked at clang-analyze for a while, but maybe there finally is a way to exclude results using an external file (rather than augment this inline). >> if (getsockname(sock->fd, (struct sockaddr *) &local, &local_size) < 0) { >> VLOG_ERR("getsockname: %s", ovs_strerror(errno)); >> goto error; > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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;
The Clang analyzer will alert on the use of uninitialized variable local despite the fact that this should be set by a syscall. To suppress the warning, this variable is now initialized. Signed-off-by: Mike Pattrick <mkp@redhat.com> --- lib/netlink-socket.c | 1 + 1 file changed, 1 insertion(+)