Changed the way we build the HMSET command such that we don't

continue to destroy and reallocate the command buffer

Added a simply library routine to append to a command buffer
using a smart_str

Made the unit tests work even if you're not compiled with
igbinary

Addresses issue phpredis#287

Author: michael-grunder

library.c

@@ -447,6 +447,20 @@ int redis_cmd_append_str(char **cmd, int cmd_len, char *append, int append_len)
 	return buf.len;
 }
 
+/*
+ * Append a command sequence to a smart_str
+ */
+int redis_cmd_append_sstr(smart_str *str, char *append, int append_len) {
+    smart_str_appendc(str, '$');
+    smart_str_append_long(str, append_len);
+    smart_str_appendl(str, _NL, sizeof(_NL) - 1);
+    smart_str_appendl(str, append, append_len);
+    smart_str_appendl(str, _NL, sizeof(_NL) - 1);
+
+    // Return our new length
+    return str->len;
+}
+
 /*
  * Append an integer command to a Redis command
  */

library.h

@@ -4,6 +4,7 @@ int redis_cmd_format(char **ret, char *format, ...);
 int redis_cmd_format_static(char **ret, char *keyword, char *format, ...);
 int redis_cmd_format_header(char **ret, char *keyword, int arg_count);
 int redis_cmd_append_str(char **cmd, int cmd_len, char *append, int append_len);
+int redis_cmd_append_sstr(smart_str *str, char *append, int append_len);
 int redis_cmd_append_int(char **cmd, int cmd_len, int append);
 
 

redis.c

@@ -4856,18 +4856,15 @@ PHP_METHOD(Redis, hMget) {
 	REDIS_PROCESS_RESPONSE_CLOSURE(redis_sock_read_multibulk_reply_assoc, z_keys);
 }
 
-
 PHP_METHOD(Redis, hMset)
 {
     zval *object;
     RedisSock *redis_sock;
-    char *key = NULL, *cmd;
-    int key_len, cmd_len, key_free;
+    char *key = NULL, *cmd, *old_cmd = NULL;
+    int key_len, cmd_len, key_free, i, element_count = 2;
     zval *z_hash;
     HashTable *ht_hash;
-    int i;
-    int element_count = 2;
-    char *old_cmd = NULL;
+    smart_str set_cmds = {0};
 
     if (zend_parse_method_parameters(ZEND_NUM_ARGS() TSRMLS_CC, getThis(), "Osa",
                                      &object, redis_ce,
@@ -4885,68 +4882,63 @@ PHP_METHOD(Redis, hMset)
         RETURN_FALSE;
     }
 
-	key_free = redis_key_prefix(redis_sock, &key, &key_len TSRMLS_CC);
-    cmd_len = redis_cmd_format(&cmd, 
+    key_free = redis_key_prefix(redis_sock, &key, &key_len TSRMLS_CC);
+    cmd_len = redis_cmd_format(&cmd,
                     "$5" _NL "HMSET" _NL
                     "$%d" _NL "%s" _NL
                     , key_len, key, key_len);
-	if(key_free) efree(key);
+    if(key_free) efree(key);
 
     /* looping on each item of the array */
-	for(i =0, zend_hash_internal_pointer_reset(ht_hash);
-		zend_hash_has_more_elements(ht_hash) == SUCCESS;
-		i++, zend_hash_move_forward(ht_hash)) {
-
-		char *hkey;
-		unsigned int hkey_len;
-		unsigned long idx;
-		int type;
-		int hkey_free = 0;
-		zval **z_value_p;
+    for(i =0, zend_hash_internal_pointer_reset(ht_hash);
+        zend_hash_has_more_elements(ht_hash) == SUCCESS;
+        i++, zend_hash_move_forward(ht_hash)) {
+
+        char *hkey, hkey_str[40];
+        unsigned int hkey_len;
+        unsigned long idx;
+        int type;
+        zval **z_value_p;
 
-		char *hval;
+        char *hval;
         int hval_len, hval_free;
 
-		type = zend_hash_get_current_key_ex(ht_hash, &hkey, &hkey_len, &idx, 0, NULL);
+        type = zend_hash_get_current_key_ex(ht_hash, &hkey, &hkey_len, &idx, 0, NULL);
 
-		if(zend_hash_get_current_data(ht_hash, (void**)&z_value_p) == FAILURE) {
-			continue; 	/* this should never happen */
-		}
+        if(zend_hash_get_current_data(ht_hash, (void**)&z_value_p) == FAILURE) {
+            continue;   /* this should never happen */
+        }
 
-		if(type != HASH_KEY_IS_STRING) { /* convert to string */
-			hkey_free = 1;
-			hkey = emalloc(40);
-			hkey_len = 1 + sprintf(hkey, "%ld", idx);
+        if(type != HASH_KEY_IS_STRING) { /* convert to string */
+            hkey_len = 1 + sprintf(hkey_str, "%ld", idx);
+            hkey = (char*)hkey_str;
         }
         element_count += 2;
 
         /* key is set. */
         hval_free = redis_serialize(redis_sock, *z_value_p, &hval, &hval_len TSRMLS_CC);
 
-        old_cmd = cmd;
-        cmd_len = redis_cmd_format(&cmd, "%s"
-                        "$%d" _NL "%s" _NL
-                        "$%d" _NL "%s" _NL
-                        , cmd, cmd_len
-                        , hkey_len-1, hkey, hkey_len-1
-                        , hval_len, hval, hval_len);
-        efree(old_cmd);
+        // Append our member and value in place
+        redis_cmd_append_sstr(&set_cmds, hkey, hkey_len - 1);
+        redis_cmd_append_sstr(&set_cmds, hval, hval_len);
+
         if(hval_free) efree(hval);
-        if(hkey_free) efree(hkey);
     }
 
+    // Now construct the entire command
     old_cmd = cmd;
-    cmd_len = redis_cmd_format(&cmd, "*%d" _NL "%s"
-                    , element_count, cmd, cmd_len);
+    cmd_len = redis_cmd_format(&cmd, "*%d" _NL "%s%s", element_count, cmd, cmd_len, set_cmds.c, set_cmds.len);
     efree(old_cmd);
 
-	REDIS_PROCESS_REQUEST(redis_sock, cmd, cmd_len);
-	IF_ATOMIC() {
-		redis_boolean_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
-	}
-	REDIS_PROCESS_RESPONSE(redis_boolean_response);
-}
+    // Free the HMSET bits
+    efree(set_cmds.c);
 
+    REDIS_PROCESS_REQUEST(redis_sock, cmd, cmd_len);
+    IF_ATOMIC() {
+        redis_boolean_response(INTERNAL_FUNCTION_PARAM_PASSTHRU, redis_sock, NULL, NULL);
+    }
+    REDIS_PROCESS_RESPONSE(redis_boolean_response);
+}
 
 
 PHPAPI int redis_response_enqueued(RedisSock *redis_sock TSRMLS_DC) {

tests/TestRedis.php

@@ -3263,7 +3263,10 @@ public function testUnserialize() {
     		1,1.5,'one',Array('this','is','an','array')
     	);
 
-    	foreach(Array(Redis::SERIALIZER_PHP, Redis::SERIALIZER_IGBINARY) as $mode) {
+        $serializers = Array(Redis::SERIALIZER_PHP);
+        if(defined('Redis::SERIALIZER_IGBINARY')) $serializers[] = Redis::SERIALIZER_IGBINARY;
+
+    	foreach($serializers as $mode) {
     		$vals_enc = Array();
 
     		// Pass them through redis so they're serialized