* Only attempt to set TCP_NODELAY on TCP sockets and also don't abort

if we can't. * Stop throwing warnings if users double up on pipeline or multi calls reverting to previous behavior.
wl6179 · Aug 14, 2017 · d11798e · d11798e
1 parent e672f40
commit d11798e
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 20 deletions.
diff --git a/library.c b/library.c
@@ -1411,7 +1411,7 @@ PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC)
     struct timeval tv, read_tv, *tv_ptr = NULL;
     char host[1024], *persistent_id = NULL;
     const char *fmtstr = "%s:%d";
-    int host_len, err = 0;
+    int host_len, usocket = 0, err = 0;
     php_netstream_data_t *sock;
     int tcp_flag = 1;
 
@@ -1430,6 +1430,7 @@ PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC)
 
     if(redis_sock->host[0] == '/' && redis_sock->port < 1) {
         host_len = snprintf(host, sizeof(host), "unix://%s", redis_sock->host);
+        usocket = 1;
     } else {
         if(redis_sock->port == 0)
             redis_sock->port = 6379;
@@ -1466,10 +1467,10 @@ PHP_REDIS_API int redis_sock_connect(RedisSock *redis_sock TSRMLS_DC)
         return -1;
     }
 
-    /* set TCP_NODELAY */
+    /* Attempt to set TCP_NODELAY if we're not using a unix socket */
     sock = (php_netstream_data_t*)redis_sock->stream->abstract;
-    if (setsockopt(sock->socket, IPPROTO_TCP, TCP_NODELAY, (char *) &tcp_flag, sizeof(int)) < 0) {
-        php_error_docref(NULL TSRMLS_CC, E_ERROR, "Can't activate TCP_NODELAY option!");
+    if (!usocket) {
+        setsockopt(sock->socket, IPPROTO_TCP, TCP_NODELAY, (char *) &tcp_flag, sizeof(int));
     }
 
     php_stream_auto_cleanup(redis_sock->stream);

diff --git a/redis.c b/redis.c
@@ -2605,19 +2605,20 @@ PHP_METHOD(Redis, multi)
     }
 
     if (multi_value == PIPELINE) {
-        IF_PIPELINE() {
-            php_error_docref(NULL TSRMLS_CC, E_WARNING, "Already in pipeline mode");
-        } else IF_MULTI() {
+        /* Cannot enter pipeline mode in a MULTI block */
+        IF_MULTI() {
             php_error_docref(NULL TSRMLS_CC, E_ERROR, "Can't activate pipeline in multi mode!");
             RETURN_FALSE;
-        } else {
+        }
+
+        /* Enable PIPELINE if we're not already in one */
+        IF_ATOMIC() {
             free_reply_callbacks(redis_sock);
             REDIS_ENABLE_MODE(redis_sock, PIPELINE);
         }
     } else if (multi_value == MULTI) {
-        IF_MULTI() {
-            php_error_docref(NULL TSRMLS_CC, E_WARNING, "Already in multi mode");
-        } else {
+        /* Don't want to do anything if we're alredy in MULTI mode */
+        IF_NOT_MULTI() {
             cmd_len = REDIS_SPPRINTF(&cmd, "MULTI", "");
             IF_PIPELINE() {
                 PIPELINE_ENQUEUE_COMMAND(cmd, cmd_len);
@@ -2638,8 +2639,10 @@ PHP_METHOD(Redis, multi)
             }
         }
     } else {
+        php_error_docref(NULL TSRMLS_CC, E_WARNING, "Unknown mode sent to Redis::multi");
         RETURN_FALSE;
     }
+
     RETURN_ZVAL(getThis(), 1, 0);
 }
 
@@ -2821,21 +2824,22 @@ PHP_METHOD(Redis, pipeline)
         RETURN_FALSE;
     }
 
-    IF_PIPELINE() {
-        php_error_docref(NULL TSRMLS_CC, E_WARNING,
-            "Already in pipeline mode");
-    } else {
-        IF_MULTI() {
-            php_error_docref(NULL TSRMLS_CC, E_ERROR,
-                "Can't activate pipeline in multi mode!");
-            RETURN_FALSE;
-        }
+    /* User cannot enter MULTI mode if already in a pipeline */
+    IF_MULTI() {
+        php_error_docref(NULL TSRMLS_CC, E_ERROR, "Can't activate pipeline in multi mode!");
+        RETURN_FALSE;
+    }
+
+    /* Enable pipeline mode unless we're already in that mode in which case this
+     * is just a NO OP */
+    IF_ATOMIC() {
         /* NB : we keep the function fold, to detect the last function.
          * We need the response format of the n - 1 command. So, we can delete
          * when n > 2, the { 1 .. n - 2} commands */
         free_reply_callbacks(redis_sock);
         REDIS_ENABLE_MODE(redis_sock, PIPELINE);
     }
+
     RETURN_ZVAL(getThis(), 1, 0);
 }