Bob, On Thu, Feb 11, 2016 at 6:59 PM, Bob Peterson <rpete...@redhat.com> wrote: > ----- Original Message ----- >> > +out_err: >> > + sock_release(sock); >> > + sock = NULL; >> > + con->sock = NULL; >> >> Consolidating the error paths makes sense, but con->sock shouldn't be >> set here at all; the caller does that in add_sock(). > > I disagree. > > The caller doesn't call add_sock() in the error case, which is where > we're setting con->sock to NULL.
well, the caller sets con->sock on success, so why doesn't it set it on failure as well? Andreas --- fs/dlm/lowcomms.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/fs/dlm/lowcomms.c b/fs/dlm/lowcomms.c index ac7eae4..0d6e374 100644 --- a/fs/dlm/lowcomms.c +++ b/fs/dlm/lowcomms.c @@ -1265,7 +1265,6 @@ create_out: out_err: sock_release(sock); sock = NULL; - con->sock = NULL; goto create_out; } @@ -1352,7 +1351,6 @@ static int tcp_listen_for_all(void) { struct socket *sock = NULL; struct connection *con = nodeid2con(0, GFP_NOFS); - int result = -EINVAL; if (!con) return -ENOMEM; @@ -1369,13 +1367,11 @@ static int tcp_listen_for_all(void) sock = tcp_create_listen_sock(con, dlm_local_addr[0]); if (sock) { add_sock(sock, con); - result = 0; - } - else { - result = -EADDRINUSE; + return 0; + } else { + con->sock = NULL; + return -EADDRINUSE; } - - return result; } -- 2.5.0