| Message ID | 1473109698-31408-20-git-send-email-jgunthorpe@obsidianresearch.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 9/6/2016 12:08 AM, Jason Gunthorpe wrote: > gcc remarks: > > ../libibverbs/src/neigh.c:339:6: warning: 'sock_fd' may be used uninitialized in this function [-Wmaybe-uninitialized] > err = try_send_to(sock_fd, buff, sizeof(buff), &addr_dst); Why not just initializing 'sock_fd' to any value (e.g. to 0) when it's declared and drop all below patch ? as the code checks the return code from create_socket() it should be safe. > But this is bogus because create_socket will always return > an error if it does not set psock_fd. It looks like the > insane if logic is just a tish too much for gcc to handle. > > Since the result of create_socket is discarded anyhow, simplify the > tortured logic. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > libibverbs/src/neigh.c | 12 +++++------- > 1 file changed, 5 insertions(+), 7 deletions(-) > > diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c > index 799b810a9ec4..dc8c2bc99ae3 100644 > --- a/libibverbs/src/neigh.c > +++ b/libibverbs/src/neigh.c > @@ -207,7 +207,7 @@ static int create_socket(struct get_neigh_handler *neigh_handler, > &addr_src.len); > if (err) { > errno = EADDRNOTAVAIL; > - return err; > + return -1; > } > > addr_dst->len = sizeof(addr_dst->sktaddr); > @@ -216,24 +216,22 @@ static int create_socket(struct get_neigh_handler *neigh_handler, > &addr_dst->len); > if (err) { > errno = EADDRNOTAVAIL; > - return err; > + return -1; > } > > err = set_link_port(&addr_dst->sktaddr, PORT_DISCARD, > neigh_handler->oif); > if (err) > - return err; > + return -1; > > sock_fd = socket(addr_dst->sktaddr.s.sa_family, > SOCK_DGRAM | SOCK_CLOEXEC, 0); > if (sock_fd == -1) > - return errno ? -errno : -1; > + return -1; > err = bind(sock_fd, &addr_src.sktaddr.s, addr_src.len); > if (err) { > - int bind_err = -errno; > - > close(sock_fd); > - return bind_err ?: EADDRNOTAVAIL; > + return -1; > } > > *psock_fd = sock_fd; > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Sep 14, 2016 at 07:46:15PM +0300, Yishai Hadas wrote: > On 9/6/2016 12:08 AM, Jason Gunthorpe wrote: > >gcc remarks: > > > >../libibverbs/src/neigh.c:339:6: warning: 'sock_fd' may be used uninitialized in this function [-Wmaybe-uninitialized] > > err = try_send_to(sock_fd, buff, sizeof(buff), &addr_dst); > > > Why not just initializing 'sock_fd' to any value (e.g. to 0) when it's > declared and drop all below patch ? as the code checks the return code from > create_socket() it should be safe. Because that is a lazy fix. This code is full of cruft computing an error value that is never used. There is no reason to do that so the best approach is to remove the dead code cruft. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/libibverbs/src/neigh.c b/libibverbs/src/neigh.c index 799b810a9ec4..dc8c2bc99ae3 100644 --- a/libibverbs/src/neigh.c +++ b/libibverbs/src/neigh.c @@ -207,7 +207,7 @@ static int create_socket(struct get_neigh_handler *neigh_handler, &addr_src.len); if (err) { errno = EADDRNOTAVAIL; - return err; + return -1; } addr_dst->len = sizeof(addr_dst->sktaddr); @@ -216,24 +216,22 @@ static int create_socket(struct get_neigh_handler *neigh_handler, &addr_dst->len); if (err) { errno = EADDRNOTAVAIL; - return err; + return -1; } err = set_link_port(&addr_dst->sktaddr, PORT_DISCARD, neigh_handler->oif); if (err) - return err; + return -1; sock_fd = socket(addr_dst->sktaddr.s.sa_family, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (sock_fd == -1) - return errno ? -errno : -1; + return -1; err = bind(sock_fd, &addr_src.sktaddr.s, addr_src.len); if (err) { - int bind_err = -errno; - close(sock_fd); - return bind_err ?: EADDRNOTAVAIL; + return -1; } *psock_fd = sock_fd;
gcc remarks: ../libibverbs/src/neigh.c:339:6: warning: 'sock_fd' may be used uninitialized in this function [-Wmaybe-uninitialized] err = try_send_to(sock_fd, buff, sizeof(buff), &addr_dst); But this is bogus because create_socket will always return an error if it does not set psock_fd. It looks like the insane if logic is just a tish too much for gcc to handle. Since the result of create_socket is discarded anyhow, simplify the tortured logic. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- libibverbs/src/neigh.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)