Add a custom MPOP handler for RedisClusterlmpop-zmpop

5 files changed, 70 insertions, 32 deletions

diff --git a/cluster_library.c b/cluster_library.c
index 6e69c518..bb682608 100644
--- a/cluster_library.c
+++ b/cluster_library.c
@@ -2362,6 +2362,23 @@ cluster_xinfo_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx)
     add_next_index_zval(&c->multi_resp, &z_ret);
 }
 
+/* LMPOP, ZMPOP, BLMPOP, BZMPOP */
+PHP_REDIS_API void
+cluster_mpop_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx)
+{
+    zval z_ret;
+
+    c->cmd_sock->null_mbulk_as_null = c->flags->null_mbulk_as_null;
+    if (redis_read_mpop_response(c->cmd_sock, &z_ret, c->reply_len, ctx) == FAILURE) {
+        CLUSTER_RETURN_FALSE(c);
+    }
+
+    if (CLUSTER_IS_ATOMIC(c)) {
+        RETURN_ZVAL(&z_ret, 0, 0);
+    }
+    add_next_index_zval(&c->multi_resp, &z_ret);
+}
+
 static void
 cluster_acl_custom_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx,
                         int (*cb)(RedisSock*, zval*, long))
diff --git a/cluster_library.h b/cluster_library.h
index ddb29f29..1f18c35a 100644
--- a/cluster_library.h
+++ b/cluster_library.h
@@ -496,6 +496,9 @@ PHP_REDIS_API void cluster_xclaim_resp(INTERNAL_FUNCTION_PARAMETERS,
 PHP_REDIS_API void cluster_xinfo_resp(INTERNAL_FUNCTION_PARAMETERS,
     redisCluster *c, void *ctx);
 
+PHP_REDIS_API void cluster_mpop_resp(INTERNAL_FUNCTION_PARAMETERS,
+    redisCluster *c, void *ctx);
+
 /* Custom ACL handlers */
 PHP_REDIS_API void cluster_acl_getuser_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx);
 PHP_REDIS_API void cluster_acl_log_resp(INTERNAL_FUNCTION_PARAMETERS, redisCluster *c, void *ctx);
diff --git a/library.c b/library.c
index 4c8817b5..09b31e6c 100644
--- a/library.c
+++ b/library.c
@@ -68,6 +68,17 @@
 #define SCORE_DECODE_INT  1
 #define SCORE_DECODE_DOUBLE 2
 
+/* PhpRedis often returns either FALSE or NULL depending on whether we have
+ * an option set, so this macro just wraps that often repeated logic */
+#define REDIS_ZVAL_NULL(sock_, zv_) \
+    do { \
+        if ((sock_)->null_mbulk_as_null) { \
+            ZVAL_NULL((zv_)); \
+        } else { \
+            ZVAL_FALSE((zv_)); \
+        } \
+    } while (0)
+
 #ifndef PHP_WIN32
     #include <netinet/tcp.h> /* TCP_NODELAY */
     #include <sys/socket.h>  /* SO_KEEPALIVE */
@@ -1612,30 +1623,24 @@ geosearch_cast(zval *zv)
 }
 
 PHP_REDIS_API int
-redis_mpop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
-                    zval *z_tab, void *ctx)
+redis_read_mpop_response(RedisSock *redis_sock, zval *zdst, int elements,
+                         void *ctx)
 {
-    zval zret = {0}, zele = {0};
-    int elements, subele, keylen;
+    int subele, keylen;
+    zval zele = {0};
     char *key;
 
     ZEND_ASSERT(ctx == NULL || ctx == PHPREDIS_CTX_PTR);
 
-    if (read_mbulk_header(redis_sock, &elements) == FAILURE) {
-        goto fail;
-    } else if (elements < 0) {
-        if (redis_sock->null_mbulk_as_null) {
-            ZVAL_NULL(&zret);
-        } else {
-            ZVAL_FALSE(&zret);
-        }
-        goto exit;
+    if (elements < 0) {
+        REDIS_ZVAL_NULL(redis_sock, zdst);
+        return SUCCESS;
     }
 
     /* Invariant:  We should have two elements */
     ZEND_ASSERT(elements == 2);
 
-    array_init(&zret);
+    array_init(zdst);
 
     /* Key name and number of entries */
     if ((key = redis_sock_read(redis_sock, &keylen)) == NULL ||
@@ -1645,7 +1650,7 @@ redis_mpop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
         goto fail;
     }
 
-    add_next_index_stringl(&zret, key, keylen);
+    add_next_index_stringl(zdst, key, keylen);
     efree(key);
 
     array_init_size(&zele, elements);
@@ -1664,28 +1669,38 @@ redis_mpop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
         redis_mbulk_reply_loop(redis_sock, &zele, elements, UNSERIALIZE_ALL);
     }
 
-    add_next_index_zval(&zret, &zele);
-
-exit:
-
-    if (IS_ATOMIC(redis_sock)) {
-        RETVAL_ZVAL(&zret, 0, 0);
-    } else {
-        add_next_index_zval(z_tab, &zret);
-    }
+    add_next_index_zval(zdst, &zele);
 
     return SUCCESS;
 
 fail:
-    zval_dtor(&zret);
+    zval_dtor(zdst);
+    ZVAL_FALSE(zdst);
+
+    return FAILURE;
+}
+
+PHP_REDIS_API int
+redis_mpop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
+                    zval *z_tab, void *ctx)
+{
+    int elements, res = SUCCESS;
+    zval zret = {0};
+
+    if (read_mbulk_header(redis_sock, &elements) == FAILURE ||
+        redis_read_mpop_response(redis_sock, &zret, elements, ctx) == FAILURE)
+    {
+        res = FAILURE;
+        ZVAL_FALSE(&zret);
+    }
 
     if (IS_ATOMIC(redis_sock)) {
-        RETVAL_FALSE;
+        RETVAL_ZVAL(&zret, 0, 0);
     } else {
-        add_next_index_bool(z_tab, 0);
+        add_next_index_zval(z_tab, &zret);
     }
 
-    return FAILURE;
+    return res;
 }
 
 PHP_REDIS_API int
diff --git a/library.h b/library.h
index 8132f3e6..ded5d3c8 100644
--- a/library.h
+++ b/library.h
@@ -152,6 +152,9 @@ PHP_REDIS_API int
 redis_mpop_response(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock,
                     zval *z_tab, void *ctx);
 
+PHP_REDIS_API int
+redis_read_mpop_response(RedisSock *redis_sock, zval *zdst, int elements, void *ctx);
+
 /* Specialized ACL reply handlers */
 PHP_REDIS_API int redis_read_acl_getuser_reply(RedisSock *redis_sock, zval *zret, long len);
 PHP_REDIS_API int redis_acl_getuser_reply(INTERNAL_FUNCTION_PARAMETERS, RedisSock *redis_sock, zval *z_tab, void *ctx);
diff --git a/redis_cluster.c b/redis_cluster.c
index 4e6af51f..9d466bde 100644
--- a/redis_cluster.c
+++ b/redis_cluster.c
@@ -1338,25 +1338,25 @@ PHP_METHOD(RedisCluster, lcs) {
 
 /* {{{ proto Redis|array|false Redis::lmpop(array $keys, string $from, int $count = 1) */
 PHP_METHOD(RedisCluster, lmpop) {
-    CLUSTER_PROCESS_KW_CMD("LMPOP", redis_mpop_cmd, cluster_variant_resp, 0);
+    CLUSTER_PROCESS_KW_CMD("LMPOP", redis_mpop_cmd, cluster_mpop_resp, 0);
 }
 /* }}} */
 
 /* {{{ proto Redis|array|false Redis::blmpop(double $timeout, array $keys, string $from, int $count = 1) */
 PHP_METHOD(RedisCluster, blmpop) {
-    CLUSTER_PROCESS_KW_CMD("BLMPOP", redis_mpop_cmd, cluster_variant_resp, 0);
+    CLUSTER_PROCESS_KW_CMD("BLMPOP", redis_mpop_cmd, cluster_mpop_resp, 0);
 }
 /* }}} */
 
 /* {{{ proto Redis|array|false Redis::zmpop(array $keys, string $from, int $count = 1) */
 PHP_METHOD(RedisCluster, zmpop) {
-    CLUSTER_PROCESS_KW_CMD("ZMPOP", redis_mpop_cmd, cluster_variant_resp, 0);
+    CLUSTER_PROCESS_KW_CMD("ZMPOP", redis_mpop_cmd, cluster_mpop_resp, 0);
 }
 /* }}} */
 
 /* {{{ proto Redis|array|false Redis::bzmpop(double $timeout, array $keys, sring $from, int $count = 1) */
 PHP_METHOD(RedisCluster, bzmpop) {
-    CLUSTER_PROCESS_KW_CMD("BZMPOP", redis_mpop_cmd, cluster_variant_resp, 0);
+    CLUSTER_PROCESS_KW_CMD("BZMPOP", redis_mpop_cmd, cluster_mpop_resp, 0);
 }
 /* }}} */