From 2c99b7db00a6238fd43053dd672c8ce519d8fd27 Mon Sep 17 00:00:00 2001

From: Xavi Hernandez <xhernandez@redhat.com>

Date: Wed, 11 Dec 2019 18:21:14 +0100

Subject: [PATCH 341/344] socket: fix error handling

When __socket_proto_state_machine() detected a problem in the size of

the request or it couldn't allocate an iobuf of the requested size, it

returned -ENOMEM (-12). However the caller was expecting only -1 in

case of error. For this reason the error passes undetected initially,

adding back the socket to the epoll object. On further processing,

however, the error is finally detected and the connection terminated.

Meanwhile, another thread could receive a poll_in event from the same

connection, which could cause races with the connection destruction.

When this happened, the process crashed.

To fix this, all error detection conditions have been hardened to be

more strict on what is valid and what not. Also, we don't return

-ENOMEM anymore. We always return -1 in case of error.

An additional change has been done to prevent destruction of the

transport object while it may still be needed.

Upstream patch:

> Change-Id: I6e59cd81cbf670f7adfdde942625d4e6c3fbc82d

> Upstream patch link: https://review.gluster.org/c/glusterfs/+/23861

> Fixes: bz#1782495

> Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>

Change-Id: I6e59cd81cbf670f7adfdde942625d4e6c3fbc82d

BUG: 1779696

Signed-off-by: Xavi Hernandez <xhernandez@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/187689

Tested-by: RHGS Build Bot <nigelb@redhat.com>

Reviewed-by: Raghavendra Gowdappa <rgowdapp@redhat.com>

---

 rpc/rpc-transport/socket/src/socket.c | 173 ++++++++++++++++++----------------

 1 file changed, 90 insertions(+), 83 deletions(-)

diff --git a/rpc/rpc-transport/socket/src/socket.c b/rpc/rpc-transport/socket/src/socket.c

index bf2fa71..f54ca83 100644

--- a/rpc/rpc-transport/socket/src/socket.c

+++ b/rpc/rpc-transport/socket/src/socket.c

@@ -173,7 +173,7 @@ ssl_setup_connection_params(rpc_transport_t *this);

                                                                                \

         ret = __socket_readv(this, in->pending_vector, 1, &in->pending_vector, \

                              &in->pending_count, &bytes_read);                 \

-        if (ret == -1)                                                         \

+        if (ret < 0)                                                           \

             break;                                                             \

         __socket_proto_update_priv_after_read(priv, ret, bytes_read);          \

     }

@@ -739,7 +739,7 @@ __socket_rwv(rpc_transport_t *this, struct iovec *vector, int count,

                 ret = sys_writev(sock, opvector, IOV_MIN(opcount));

             }

-            if (ret == 0 || (ret == -1 && errno == EAGAIN)) {

+            if ((ret == 0) || ((ret < 0) && (errno == EAGAIN))) {

                 /* done for now */

                 break;

             } else if (ret > 0)

@@ -754,7 +754,7 @@ __socket_rwv(rpc_transport_t *this, struct iovec *vector, int count,

                 errno = ENODATA;

                 ret = -1;

             }

-            if (ret == -1 && errno == EAGAIN) {

+            if ((ret < 0) && (errno == EAGAIN)) {

                 /* done for now */

                 break;

             } else if (ret > 0)

@@ -770,7 +770,7 @@ __socket_rwv(rpc_transport_t *this, struct iovec *vector, int count,

             errno = ENOTCONN;

             break;

         }

-        if (ret == -1) {

+        if (ret < 0) {

             if (errno == EINTR)

                 continue;

@@ -907,7 +907,7 @@ __socket_disconnect(rpc_transport_t *this)

     gf_log(this->name, GF_LOG_TRACE, "disconnecting %p, sock=%d", this,

            priv->sock);

-    if (priv->sock != -1) {

+    if (priv->sock >= 0) {

         gf_log_callingfn(this->name, GF_LOG_TRACE,

                          "tearing down socket connection");

         ret = __socket_teardown_connection(this);

@@ -942,7 +942,7 @@ __socket_server_bind(rpc_transport_t *this)

     ret = setsockopt(priv->sock, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt));

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log(this->name, GF_LOG_ERROR,

                "setsockopt() for SO_REUSEADDR failed (%s)", strerror(errno));

     }

@@ -955,7 +955,7 @@ __socket_server_bind(rpc_transport_t *this)

         if (reuse_check_sock >= 0) {

             ret = connect(reuse_check_sock, SA(&unix_addr),

                           this->myinfo.sockaddr_len);

-            if ((ret == -1) && (ECONNREFUSED == errno)) {

+            if ((ret != 0) && (ECONNREFUSED == errno)) {

                 sys_unlink(((struct sockaddr_un *)&unix_addr)->sun_path);

             }

             gf_log(this->name, GF_LOG_INFO,

@@ -967,7 +967,7 @@ __socket_server_bind(rpc_transport_t *this)

     ret = bind(priv->sock, (struct sockaddr *)&this->myinfo.sockaddr,

                this->myinfo.sockaddr_len);

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log(this->name, GF_LOG_ERROR, "binding to %s failed: %s",

                this->myinfo.identifier, strerror(errno));

         if (errno == EADDRINUSE) {

@@ -976,7 +976,7 @@ __socket_server_bind(rpc_transport_t *this)

     }

     if (AF_UNIX != SA(&this->myinfo.sockaddr)->sa_family) {

         if (getsockname(priv->sock, SA(&this->myinfo.sockaddr),

-                        &this->myinfo.sockaddr_len) == -1) {

+                        &this->myinfo.sockaddr_len) != 0) {

             gf_log(this->name, GF_LOG_WARNING,

                    "getsockname on (%d) failed (%s)", priv->sock,

                    strerror(errno));

@@ -1004,7 +1004,7 @@ __socket_nonblock(int fd)

     flags = fcntl(fd, F_GETFL);

-    if (flags != -1)

+    if (flags >= 0)

         ret = fcntl(fd, F_SETFL, flags | O_NONBLOCK);

     return ret;

@@ -1034,7 +1034,7 @@ __socket_keepalive(int fd, int family, int keepaliveintvl, int keepaliveidle,

 #endif

     ret = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &on, sizeof(on));

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log("socket", GF_LOG_WARNING,

                "failed to set keep alive option on socket %d", fd);

         goto err;

@@ -1051,7 +1051,7 @@ __socket_keepalive(int fd, int family, int keepaliveintvl, int keepaliveidle,

     ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPALIVE, &keepaliveintvl,

                      sizeof(keepaliveintvl));

 #endif

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log("socket", GF_LOG_WARNING,

                "failed to set keep alive interval on socket %d", fd);

         goto err;

@@ -1062,7 +1062,7 @@ __socket_keepalive(int fd, int family, int keepaliveintvl, int keepaliveidle,

     ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &keepaliveidle,

                      sizeof(keepaliveidle));

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log("socket", GF_LOG_WARNING,

                "failed to set keep idle %d on socket %d, %s", keepaliveidle, fd,

                strerror(errno));

@@ -1070,7 +1070,7 @@ __socket_keepalive(int fd, int family, int keepaliveintvl, int keepaliveidle,

     }

     ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPINTVL, &keepaliveintvl,

                      sizeof(keepaliveintvl));

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log("socket", GF_LOG_WARNING,

                "failed to set keep interval %d on socket %d, %s",

                keepaliveintvl, fd, strerror(errno));

@@ -1082,7 +1082,7 @@ __socket_keepalive(int fd, int family, int keepaliveintvl, int keepaliveidle,

         goto done;

     ret = setsockopt(fd, IPPROTO_TCP, TCP_USER_TIMEOUT, &timeout_ms,

                      sizeof(timeout_ms));

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log("socket", GF_LOG_WARNING,

                "failed to set "

                "TCP_USER_TIMEOUT %d on socket %d, %s",

@@ -1093,7 +1093,7 @@ __socket_keepalive(int fd, int family, int keepaliveintvl, int keepaliveidle,

 #if defined(TCP_KEEPCNT)

     ret = setsockopt(fd, IPPROTO_TCP, TCP_KEEPCNT, &keepalivecnt,

                      sizeof(keepalivecnt));

-    if (ret == -1) {

+    if (ret != 0) {

         gf_log("socket", GF_LOG_WARNING,

                "failed to set "

                "TCP_KEEPCNT %d on socket %d, %s",

@@ -1366,7 +1366,7 @@ socket_event_poll_err(rpc_transport_t *this, int gen, int idx)

     pthread_mutex_lock(&priv->out_lock);

     {

-        if ((priv->gen == gen) && (priv->idx == idx) && (priv->sock != -1)) {

+        if ((priv->gen == gen) && (priv->idx == idx) && (priv->sock >= 0)) {

             __socket_ioq_flush(this);

             __socket_reset(this);

             socket_closed = _gf_true;

@@ -1405,7 +1405,7 @@ socket_event_poll_out(rpc_transport_t *this)

         if (priv->connected == 1) {

             ret = __socket_ioq_churn(this);

-            if (ret == -1) {

+            if (ret < 0) {

                 gf_log(this->name, GF_LOG_TRACE,

                        "__socket_ioq_churn returned -1; "

                        "disconnecting socket");

@@ -1463,7 +1463,7 @@ __socket_read_simple_msg(rpc_transport_t *this)

                                      &bytes_read);

             }

-            if (ret == -1) {

+            if (ret < 0) {

                 gf_log(this->name, GF_LOG_WARNING,

                        "reading from socket failed. Error (%s), "

                        "peer (%s)",

@@ -1661,8 +1661,8 @@ __socket_read_vectored_request(rpc_transport_t *this,

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                RPC_LASTFRAG(in->fraghdr))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              RPC_LASTFRAG(in->fraghdr))) {

                 request->vector_state = SP_STATE_VECTORED_REQUEST_INIT;

                 in->payload_vector.iov_len = ((unsigned long)frag->fragcurrent -

                                               (unsigned long)

@@ -1739,8 +1739,8 @@ __socket_read_request(rpc_transport_t *this)

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                (RPC_LASTFRAG(in->fraghdr)))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              (RPC_LASTFRAG(in->fraghdr)))) {

                 request->header_state = SP_STATE_REQUEST_HEADER_INIT;

             }

@@ -1870,8 +1870,8 @@ __socket_read_accepted_successful_reply(rpc_transport_t *this)

             /* now read the entire remaining msg into new iobuf */

             ret = __socket_read_simple_msg(this);

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                RPC_LASTFRAG(in->fraghdr))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              RPC_LASTFRAG(in->fraghdr))) {

                 frag->call_body.reply.accepted_success_state =

                     SP_STATE_ACCEPTED_SUCCESS_REPLY_INIT;

             }

@@ -2003,8 +2003,8 @@ __socket_read_accepted_successful_reply_v2(rpc_transport_t *this)

             /* now read the entire remaining msg into new iobuf */

             ret = __socket_read_simple_msg(this);

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                RPC_LASTFRAG(in->fraghdr))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              RPC_LASTFRAG(in->fraghdr))) {

                 frag->call_body.reply.accepted_success_state =

                     SP_STATE_ACCEPTED_SUCCESS_REPLY_INIT;

             }

@@ -2103,8 +2103,8 @@ __socket_read_accepted_reply(rpc_transport_t *this)

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                (RPC_LASTFRAG(in->fraghdr)))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              (RPC_LASTFRAG(in->fraghdr)))) {

                 frag->call_body.reply

                     .accepted_state = SP_STATE_ACCEPTED_REPLY_INIT;

             }

@@ -2169,8 +2169,8 @@ __socket_read_vectored_reply(rpc_transport_t *this)

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                (RPC_LASTFRAG(in->fraghdr)))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              (RPC_LASTFRAG(in->fraghdr)))) {

                 frag->call_body.reply

                     .status_state = SP_STATE_VECTORED_REPLY_STATUS_INIT;

                 in->payload_vector.iov_len = (unsigned long)frag->fragcurrent -

@@ -2237,7 +2237,7 @@ __socket_read_reply(rpc_transport_t *this)

         /* Transition back to externally visible state. */

         frag->state = SP_STATE_READ_MSGTYPE;

-        if (ret == -1) {

+        if (ret < 0) {

             gf_log(this->name, GF_LOG_WARNING,

                    "notify for event MAP_XID failed for %s",

                    this->peerinfo.identifier);

@@ -2315,8 +2315,8 @@ __socket_read_frag(rpc_transport_t *this)

             remaining_size = RPC_FRAGSIZE(in->fraghdr) - frag->bytes_read;

-            if ((ret == -1) || ((ret == 0) && (remaining_size == 0) &&

-                                (RPC_LASTFRAG(in->fraghdr)))) {

+            if ((ret < 0) || ((ret == 0) && (remaining_size == 0) &&

+                              (RPC_LASTFRAG(in->fraghdr)))) {

                 /* frag->state = SP_STATE_NADA; */

                 frag->state = SP_STATE_RPCFRAG_INIT;

             }

@@ -2400,7 +2400,7 @@ __socket_proto_state_machine(rpc_transport_t *this,

                 ret = __socket_readv(this, in->pending_vector, 1,

                                      &in->pending_vector, &in->pending_count,

                                      NULL);

-                if (ret == -1)

+                if (ret < 0)

                     goto out;

                 if (ret > 0) {

@@ -2422,7 +2422,7 @@ __socket_proto_state_machine(rpc_transport_t *this,

                 in->total_bytes_read += RPC_FRAGSIZE(in->fraghdr);

                 if (in->total_bytes_read >= GF_UNIT_GB) {

-                    ret = -ENOMEM;

+                    ret = -1;

                     goto out;

                 }

@@ -2430,7 +2430,7 @@ __socket_proto_state_machine(rpc_transport_t *this,

                     this->ctx->iobuf_pool,

                     (in->total_bytes_read + sizeof(in->fraghdr)));

                 if (!iobuf) {

-                    ret = -ENOMEM;

+                    ret = -1;

                     goto out;

                 }

@@ -2457,7 +2457,7 @@ __socket_proto_state_machine(rpc_transport_t *this,

             case SP_STATE_READING_FRAG:

                 ret = __socket_read_frag(this);

-                if ((ret == -1) ||

+                if ((ret < 0) ||

                     (frag->bytes_read != RPC_FRAGSIZE(in->fraghdr))) {

                     goto out;

                 }

@@ -2575,7 +2575,7 @@ socket_event_poll_in(rpc_transport_t *this, gf_boolean_t notify_handled)

         pthread_mutex_unlock(&priv->notify.lock);

     }

-    if (notify_handled && (ret != -1))

+    if (notify_handled && (ret >= 0))

         event_handled(ctx->event_pool, priv->sock, priv->idx, priv->gen);

     if (pollin) {

@@ -2618,10 +2618,10 @@ socket_connect_finish(rpc_transport_t *this)

         ret = __socket_connect_finish(priv->sock);

-        if (ret == -1 && errno == EINPROGRESS)

+        if ((ret < 0) && (errno == EINPROGRESS))

             ret = 1;

-        if (ret == -1 && errno != EINPROGRESS) {

+        if ((ret < 0) && (errno != EINPROGRESS)) {

             if (!priv->connect_finish_log) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "connection to %s failed (%s); "

@@ -2640,7 +2640,7 @@ socket_connect_finish(rpc_transport_t *this)

             ret = getsockname(priv->sock, SA(&this->myinfo.sockaddr),

                               &this->myinfo.sockaddr_len);

-            if (ret == -1) {

+            if (ret != 0) {

                 gf_log(this->name, GF_LOG_WARNING,

                        "getsockname on (%d) failed (%s) - "

                        "disconnecting socket",

@@ -2924,6 +2924,13 @@ socket_event_handler(int fd, int idx, int gen, void *data, int poll_in,

         return;

     }

+    /* At this point we are sure no other thread is using the transport because

+     * we cannot receive more events until we call gf_event_handled(). However

+     * this function may call gf_event_handled() in some cases. When this is

+     * done, the transport may be destroyed at any moment if another thread

+     * handled an error event. To prevent that we take a reference here. */

+    rpc_transport_ref(this);

+

     GF_VALIDATE_OR_GOTO("socket", this, out);

     GF_VALIDATE_OR_GOTO("socket", this->private, out);

     GF_VALIDATE_OR_GOTO("socket", this->xl, out);

@@ -2960,7 +2967,7 @@ socket_event_handler(int fd, int idx, int gen, void *data, int poll_in,

             if (ret > 0) {

                 gf_log(this->name, GF_LOG_TRACE,

                        "(sock:%d) returning to wait on socket", priv->sock);

-                return;

+                goto out;

             }

         } else {

             char *sock_type = (priv->is_server ? "Server" : "Client");

@@ -3015,7 +3022,7 @@ socket_event_handler(int fd, int idx, int gen, void *data, int poll_in,

     }

 out:

-    return;

+    rpc_transport_unref(this);

 }

 static void

@@ -3074,7 +3081,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

         event_handled(ctx->event_pool, fd, idx, gen);

-        if (new_sock == -1) {

+        if (new_sock < 0) {

             gf_log(this->name, GF_LOG_WARNING, "accept on %d failed (%s)",

                    priv->sock, strerror(errno));

             goto out;

@@ -3082,7 +3089,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

         if (priv->nodelay && (new_sockaddr.ss_family != AF_UNIX)) {

             ret = __socket_nodelay(new_sock);

-            if (ret == -1) {

+            if (ret != 0) {

                 gf_log(this->name, GF_LOG_WARNING,

                        "setsockopt() failed for "

                        "NODELAY (%s)",

@@ -3094,7 +3101,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

             ret = __socket_keepalive(new_sock, new_sockaddr.ss_family,

                                      priv->keepaliveintvl, priv->keepaliveidle,

                                      priv->keepalivecnt, priv->timeout);

-            if (ret == -1)

+            if (ret != 0)

                 gf_log(this->name, GF_LOG_WARNING,

                        "Failed to set keep-alive: %s", strerror(errno));

         }

@@ -3110,7 +3117,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

         }

         ret = pthread_mutex_init(&new_trans->lock, NULL);

-        if (ret == -1) {

+        if (ret != 0) {

             gf_log(this->name, GF_LOG_WARNING,

                    "pthread_mutex_init() failed: %s; closing newly accepted "

                    "socket %d",

@@ -3130,7 +3137,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

         ret = getsockname(new_sock, SA(&new_trans->myinfo.sockaddr),

                           &new_trans->myinfo.sockaddr_len);

-        if (ret == -1) {

+        if (ret != 0) {

             gf_log(this->name, GF_LOG_WARNING,

                    "getsockname on socket %d "

                    "failed (errno:%s); closing newly accepted socket",

@@ -3237,7 +3244,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

              */

             ret = rpc_transport_notify(this, RPC_TRANSPORT_ACCEPT, new_trans);

-            if (ret != -1) {

+            if (ret >= 0) {

                 new_priv->idx = event_register(

                     ctx->event_pool, new_sock, socket_event_handler, new_trans,

                     1, 0, new_trans->notify_poller_death);

@@ -3275,7 +3282,7 @@ socket_server_event_handler(int fd, int idx, int gen, void *data, int poll_in,

             rpc_transport_unref(new_trans);

         }

-        if (ret == -1) {

+        if (ret < 0) {

             gf_log(this->name, GF_LOG_WARNING, "closing newly accepted socket");

             sys_close(new_sock);

             /* this unref is to actually cause the destruction of

@@ -3406,7 +3413,7 @@ socket_connect(rpc_transport_t *this, int port)

     pthread_mutex_lock(&priv->out_lock);

     {

-        if (priv->sock != -1) {

+        if (priv->sock >= 0) {

             gf_log_callingfn(this->name, GF_LOG_TRACE,

                              "connect () called on transport "

                              "already connected");

@@ -3420,7 +3427,7 @@ socket_connect(rpc_transport_t *this, int port)

         ret = socket_client_get_remote_sockaddr(this, &sock_union.sa,

                                                 &sockaddr_len, &sa_family);

-        if (ret == -1) {

+        if (ret < 0) {

             /* logged inside client_get_remote_sockaddr */

             goto unlock;

         }

@@ -3439,7 +3446,7 @@ socket_connect(rpc_transport_t *this, int port)

         this->peerinfo.sockaddr_len = sockaddr_len;

         priv->sock = sys_socket(sa_family, SOCK_STREAM, 0);

-        if (priv->sock == -1) {

+        if (priv->sock < 0) {

             gf_log(this->name, GF_LOG_ERROR, "socket creation failed (%s)",

                    strerror(errno));

             ret = -1;

@@ -3451,7 +3458,7 @@ socket_connect(rpc_transport_t *this, int port)

          */

         if (priv->windowsize != 0) {

             if (setsockopt(priv->sock, SOL_SOCKET, SO_RCVBUF, &priv->windowsize,

-                           sizeof(priv->windowsize)) < 0) {

+                           sizeof(priv->windowsize)) != 0) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "setting receive window "

                        "size failed: %d: %d: %s",

@@ -3459,7 +3466,7 @@ socket_connect(rpc_transport_t *this, int port)

             }

             if (setsockopt(priv->sock, SOL_SOCKET, SO_SNDBUF, &priv->windowsize,

-                           sizeof(priv->windowsize)) < 0) {

+                           sizeof(priv->windowsize)) != 0) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "setting send window size "

                        "failed: %d: %d: %s",

@@ -3484,7 +3491,7 @@ socket_connect(rpc_transport_t *this, int port)

         if (priv->nodelay && (sa_family != AF_UNIX)) {

             ret = __socket_nodelay(priv->sock);

-            if (ret == -1) {

+            if (ret != 0) {

                 gf_log(this->name, GF_LOG_ERROR, "NODELAY on %d failed (%s)",

                        priv->sock, strerror(errno));

             }

@@ -3494,7 +3501,7 @@ socket_connect(rpc_transport_t *this, int port)

             ret = __socket_keepalive(priv->sock, sa_family,

                                      priv->keepaliveintvl, priv->keepaliveidle,

                                      priv->keepalivecnt, priv->timeout);

-            if (ret == -1)

+            if (ret != 0)

                 gf_log(this->name, GF_LOG_ERROR, "Failed to set keep-alive: %s",

                        strerror(errno));

         }

@@ -3516,7 +3523,7 @@ socket_connect(rpc_transport_t *this, int port)

         ret = client_bind(this, SA(&this->myinfo.sockaddr),

                           &this->myinfo.sockaddr_len, priv->sock);

-        if (ret == -1) {

+        if (ret < 0) {

             gf_log(this->name, GF_LOG_WARNING, "client bind failed: %s",

                    strerror(errno));

             goto handler;

@@ -3525,7 +3532,7 @@ socket_connect(rpc_transport_t *this, int port)

         /* make socket non-blocking for all types of sockets */

         if (!priv->bio) {

             ret = __socket_nonblock(priv->sock);

-            if (ret == -1) {

+            if (ret != 0) {

                 gf_log(this->name, GF_LOG_ERROR, "NBIO on %d failed (%s)",

                        priv->sock, strerror(errno));

                 goto handler;

@@ -3552,7 +3559,7 @@ socket_connect(rpc_transport_t *this, int port)

         connect_attempted = _gf_true;

-        if (ret == -1 && errno == ENOENT && ign_enoent) {

+        if ((ret != 0) && (errno == ENOENT) && ign_enoent) {

             gf_log(this->name, GF_LOG_WARNING,

                    "Ignore failed connection attempt on %s, (%s) ",

                    this->peerinfo.identifier, strerror(errno));

@@ -3570,7 +3577,7 @@ socket_connect(rpc_transport_t *this, int port)

             goto handler;

         }

-        if (ret == -1 && ((errno != EINPROGRESS) && (errno != ENOENT))) {

+        if ((ret != 0) && (errno != EINPROGRESS) && (errno != ENOENT)) {

             /* For unix path based sockets, the socket path is

              * cryptic (md5sum of path) and may not be useful for

              * the user in debugging so log it in DEBUG

@@ -3634,8 +3641,8 @@ socket_connect(rpc_transport_t *this, int port)

     pthread_mutex_unlock(&priv->out_lock);

 err:

-    /* if sock != -1, then cleanup is done from the event handler */

-    if (ret == -1 && sock == -1) {

+    /* if sock >= 0, then cleanup is done from the event handler */

+    if ((ret < 0) && (sock < 0)) {

         /* Cleaup requires to send notification to upper layer which

            intern holds the big_lock. There can be dead-lock situation

            if big_lock is already held by the current thread.

@@ -3689,20 +3696,20 @@ socket_listen(rpc_transport_t *this)

     }

     pthread_mutex_unlock(&priv->out_lock);

-    if (sock != -1) {

+    if (sock >= 0) {

         gf_log_callingfn(this->name, GF_LOG_DEBUG, "already listening");

         return ret;

     }

     ret = socket_server_get_local_sockaddr(this, SA(&sockaddr), &sockaddr_len,

                                            &sa_family);

-    if (ret == -1) {

+    if (ret < 0) {

         return ret;

     }

     pthread_mutex_lock(&priv->out_lock);

     {

-        if (priv->sock != -1) {

+        if (priv->sock >= 0) {

             gf_log(this->name, GF_LOG_DEBUG, "already listening");

             goto unlock;

         }

@@ -3712,7 +3719,7 @@ socket_listen(rpc_transport_t *this)

         priv->sock = sys_socket(sa_family, SOCK_STREAM, 0);

-        if (priv->sock == -1) {

+        if (priv->sock < 0) {

             gf_log(this->name, GF_LOG_ERROR, "socket creation failed (%s)",

                    strerror(errno));

             goto unlock;

@@ -3723,7 +3730,7 @@ socket_listen(rpc_transport_t *this)

          */

         if (priv->windowsize != 0) {

             if (setsockopt(priv->sock, SOL_SOCKET, SO_RCVBUF, &priv->windowsize,

-                           sizeof(priv->windowsize)) < 0) {

+                           sizeof(priv->windowsize)) != 0) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "setting receive window size "

                        "failed: %d: %d: %s",

@@ -3731,7 +3738,7 @@ socket_listen(rpc_transport_t *this)

             }

             if (setsockopt(priv->sock, SOL_SOCKET, SO_SNDBUF, &priv->windowsize,

-                           sizeof(priv->windowsize)) < 0) {

+                           sizeof(priv->windowsize)) != 0) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "setting send window size failed:"

                        " %d: %d: %s",

@@ -3741,7 +3748,7 @@ socket_listen(rpc_transport_t *this)

         if (priv->nodelay && (sa_family != AF_UNIX)) {

             ret = __socket_nodelay(priv->sock);

-            if (ret == -1) {

+            if (ret != 0) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "setsockopt() failed for NODELAY (%s)", strerror(errno));

             }

@@ -3750,7 +3757,7 @@ socket_listen(rpc_transport_t *this)

         if (!priv->bio) {

             ret = __socket_nonblock(priv->sock);

-            if (ret == -1) {

+            if (ret != 0) {

                 gf_log(this->name, GF_LOG_ERROR,

                        "NBIO on socket %d failed "

                        "(errno:%s); closing socket",

@@ -3763,7 +3770,7 @@ socket_listen(rpc_transport_t *this)

         ret = __socket_server_bind(this);

-        if ((ret == -EADDRINUSE) || (ret == -1)) {

+        if (ret < 0) {

             /* logged inside __socket_server_bind() */

             gf_log(this->name, GF_LOG_ERROR,

                    "__socket_server_bind failed;"

@@ -3779,7 +3786,7 @@ socket_listen(rpc_transport_t *this)

         ret = listen(priv->sock, priv->backlog);

-        if (ret == -1) {

+        if (ret != 0) {

             gf_log(this->name, GF_LOG_ERROR,

                    "could not set socket %d to listen mode (errno:%s); "

                    "closing socket",

@@ -4025,7 +4032,7 @@ reconfigure(rpc_transport_t *this, dict_t *options)

     priv = this->private;

     if (dict_get_str(options, "transport.socket.keepalive", &optstr) == 0) {

-        if (gf_string2boolean(optstr, &tmp_bool) == -1) {

+        if (gf_string2boolean(optstr, &tmp_bool) != 0) {

             gf_log(this->name, GF_LOG_ERROR,

                    "'transport.socket.keepalive' takes only "

                    "boolean options, not taking any action");

@@ -4094,7 +4101,7 @@ reconfigure(rpc_transport_t *this, dict_t *options)

     if (dict_get(options, "non-blocking-io")) {

         optstr = data_to_str(dict_get(options, "non-blocking-io"));

-        if (gf_string2boolean(optstr, &tmp_bool) == -1) {

+        if (gf_string2boolean(optstr, &tmp_bool) != 0) {

             gf_log(this->name, GF_LOG_ERROR,

                    "'non-blocking-io' takes only boolean options,"

                    " not taking any action");

@@ -4109,7 +4116,7 @@ reconfigure(rpc_transport_t *this, dict_t *options)

     if (!priv->bio) {

         ret = __socket_nonblock(priv->sock);

-        if (ret == -1) {

+        if (ret != 0) {

             gf_log(this->name, GF_LOG_WARNING, "NBIO on %d failed (%s)",

                    priv->sock, strerror(errno));

             goto out;

@@ -4508,7 +4515,7 @@ socket_init(rpc_transport_t *this)

     if (dict_get(this->options, "non-blocking-io")) {

         optstr = data_to_str(dict_get(this->options, "non-blocking-io"));

-        if (gf_string2boolean(optstr, &tmp_bool) == -1) {

+        if (gf_string2boolean(optstr, &tmp_bool) != 0) {

             gf_log(this->name, GF_LOG_ERROR,

                    "'non-blocking-io' takes only boolean options,"

                    " not taking any action");

@@ -4528,7 +4535,7 @@ socket_init(rpc_transport_t *this)

         optstr = data_to_str(

             dict_get(this->options, "transport.socket.nodelay"));

-        if (gf_string2boolean(optstr, &tmp_bool) == -1) {

+        if (gf_string2boolean(optstr, &tmp_bool) != 0) {

             gf_log(this->name, GF_LOG_ERROR,

                    "'transport.socket.nodelay' takes only "

                    "boolean options, not taking any action");

@@ -4559,7 +4566,7 @@ socket_init(rpc_transport_t *this)

     priv->keepalivecnt = GF_KEEPALIVE_COUNT;

     if (dict_get_str(this->options, "transport.socket.keepalive", &optstr) ==

         0) {

-        if (gf_string2boolean(optstr, &tmp_bool) == -1) {

+        if (gf_string2boolean(optstr, &tmp_bool) != 0) {

             gf_log(this->name, GF_LOG_ERROR,

                    "'transport.socket.keepalive' takes only "

                    "boolean options, not taking any action");

@@ -4609,7 +4616,7 @@ socket_init(rpc_transport_t *this)

     if (dict_get(this->options, "transport.socket.read-fail-log")) {

         optstr = data_to_str(

             dict_get(this->options, "transport.socket.read-fail-log"));

-        if (gf_string2boolean(optstr, &tmp_bool) == -1) {