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

Reply via email to