Merge a number of patches.

libmemcached may return NULL as a result if the length of the value is 0.
Make zval_from_payload defensive accept NULL as payload
Fixed memcached-api.php to match get and get by key cast token parameter.
Separated CAS tests, and new tests.

Author: Andrei Zmievski

memcached-api.php

@@ -75,9 +75,9 @@ class Memcached {
 
 	public function __construct( $persistent_id = '' ) {}
 
-	public function get( $key, &$cas_token = null, $cache_cb = null ) {}
+	public function get( $key, $cache_cb = null, &$cas_token = null ) {}
 
-	public function getByKey( $server_key, $key, $cache_cb = null ) {}
+	public function getByKey( $server_key, $key, $cache_cb = null, &$cas_token = null ) {}
 
 	public function getMulti( array $keys, &$cas_tokens = null, $flags = 0 ) {}
 

php_memcached.c

@@ -422,6 +422,7 @@ static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
 		uint32_t dummy_flags;
 		int rc;
 		memcached_return dummy_status;
+		bool return_value_set = false;
 
 		status = memcached_mget_by_key(i_obj->memc, server_key, server_key_len, &key, &key_len, 1);
 		payload = memcached_fetch(i_obj->memc, NULL, NULL, &payload_len, &flags, &status);
@@ -432,10 +433,12 @@ static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
 		/*
 		 * If payload wasn't found and we have a read-through callback, invoke it to get
 		 * the value. The callback will take care of storing the value back into memcache.
+		 * The callback will set the return value.
 		 */
 		if (payload == NULL && status == MEMCACHED_NOTFOUND && fci.size != 0) {
 			status = php_memc_do_cache_callback(getThis(), &fci, &fcc, key, key_len,
 												return_value TSRMLS_CC);
+			return_value_set = true;
 		}
 
 		(void)memcached_fetch(i_obj->memc, NULL, NULL, &dummy_length, &dummy_flags, &dummy_status);
@@ -447,8 +450,8 @@ static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
 			RETURN_FALSE;
 		}
 
-		/* payload will be NULL if the callback was invoked */
-		if (payload != NULL) {
+		/* if memcached gave a value and there was no callback, payload may be NULL */
+		if (!return_value_set) {
 			rc = php_memc_zval_from_payload(return_value, payload, payload_len, flags TSRMLS_CC);
 			free(payload);
 			if (rc < 0) {
@@ -1954,7 +1957,6 @@ static char *php_memc_zval_to_payload(zval *value, size_t *payload_len, uint32_t
 			} else {
 #endif
 				php_serialize_data_t var_hash;
-
 				PHP_VAR_SERIALIZE_INIT(var_hash);
 				php_var_serialize(&buf, &value, &var_hash TSRMLS_CC);
 				PHP_VAR_SERIALIZE_DESTROY(var_hash);
@@ -2004,8 +2006,16 @@ static char *php_memc_zval_to_payload(zval *value, size_t *payload_len, uint32_t
 
 static int php_memc_zval_from_payload(zval *value, char *payload, size_t payload_len, uint32_t flags TSRMLS_DC)
 {
-	if (payload == NULL) {
+	/*
+	   A NULL payload is completely valid if length is 0, it is simply empty.
+	 */
+	char dummy_payload[1] = { 0 };
+	if (payload == NULL && payload_len > 0) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING,
+			"Could not handle non-existing value of length %zu", payload_len);
 		return -1;
+	} else if (payload == NULL) {
+		payload = dummy_payload;
 	}
 
 	if (flags & MEMC_VAL_COMPRESSED) {
@@ -2084,8 +2094,7 @@ static int php_memc_zval_from_payload(zval *value, char *payload, size_t payload
 			double dval = zend_strtod(payload, NULL);
 			ZVAL_DOUBLE(value, dval);
 		} else if (flags & MEMC_VAL_IS_BOOL) {
-			long bval = strtol(payload, NULL, 10);
-			ZVAL_BOOL(value, bval);
+			ZVAL_BOOL(value, payload_len > 0 && payload[0] == '1');
 		} else {
 			ZVAL_STRINGL(value, payload, payload_len, 1);
 		}

tests/cas.phpt

@@ -0,0 +1,34 @@
+--TEST--
+Memcached fetch cas & set cas
+--SKIPIF--
+<?php if (!extension_loaded("memcached")) print "skip"; ?>
+--FILE--
+<?php
+$m = new Memcached();
+$m->addServer('127.0.0.1', 11211, 1);
+
+$m->delete('cas_test');
+$cas_token = null;
+
+$m->set('cas_test', 10);
+$v = $m->get('cas_test', null, $cas_token);
+
+if (is_null($cas_token)) {
+	echo "Null cas token for key: cas_test value: 10\n";
+	return;
+}
+
+$v = $m->cas($cas_token, 'cas_test', 11);
+if (!$v) {
+	echo "Error setting key: cas_test value: 11 with CAS: $cas_token\n";
+	return;
+}
+
+$v = $m->get('cas_test');
+
+if ($v !== 11) {
+	echo "Wanted cas_test to be 11, value is: ";
+	var_dump($v);
+}
+?>
+--EXPECT--

tests/cas_multi.phpt

@@ -0,0 +1,50 @@
+--TEST--
+Memcached multi fetch cas & set cas
+--SKIPIF--
+<?php if (!extension_loaded("memcached")) print "skip"; ?>
+--FILE--
+<?php
+$m = new Memcached();
+$m->addServer('127.0.0.1', 11211, 1);
+
+$data = array(
+	'cas_test_1' => 1,
+	'cas_test_2' => 2,
+);
+
+foreach ($data as $key => $v) {
+	$m->delete($key);
+}
+
+$cas_tokens = array();
+$m->setMulti($data, 10);
+$actual = $m->getMulti(array_keys($data), $cas_tokens);
+
+foreach ($data as $key => $v) {
+	if (is_null($cas_tokens[$key])) {
+		echo "missing cas token(s)\n";
+		echo "data: ";
+		var_dump($data);
+		echo "actual data: ";
+		var_dump($actual);
+		echo "cas tokens: ";
+		var_dump($cas_tokens);
+		return;
+	}
+
+	$v = $m->cas($cas_tokens[$key], $key, 11);
+	if (!$v) {
+		echo "Error setting key: $key value: 11 with CAS: ", $cas_tokens[$key], "\n";
+		return;
+	}
+	$v = $m->get($key);
+	if ($v !== 11) {
+		echo "Wanted $key to be 11, value is: ";
+		var_dump($v);
+		return;
+	}
+}
+
+
+?>
+--EXPECT--

tests/types.phpt

@@ -36,29 +36,25 @@ $data = array(
 	array('object_dummy', new testclass()),
 );
 
+foreach ($data as $key => $value) {
+	$m->delete($key);
+}
+
 foreach ($data as $types) {
 	$m->set($types[0], $types[1]);
 	$actual = $m->get($types[0]);
-	$cas = null;
-	$actual_cas = $m->get($types[0], $cas);
-	if ($types[1] !== $actual || $types[1] !== $actual_cas) {
-		if (isset($cas) && is_object($types[1])
+	if ($types[1] !== $actual) {
+		if (is_object($types[1])
 			&& $types[1] == $actual
-			&& $types[1] == $actual_cas
-			&& get_class($types[1]) == get_class($actual)
-			&& get_class($types[1]) == get_class($actual_cas)) {
+			&& get_class($types[1]) == get_class($actual)) {
 			continue;
 		}
 		echo "=== $types[0] ===\n";
 		echo "Expected: ";
 		var_dump($types[1]);
 		echo "Actual: ";
 		var_dump($actual);
-		echo "Actual CAS: ";
-		var_dump($actual_cas);
-		echo "Cas: ", $cas, "\n";
 	}
-
 }
 
 ?>

tests/types_multi.phpt

@@ -36,28 +36,24 @@ $data = array(
 	'object_dummy' => new testclass(),
 );
 
+foreach ($data as $key => $value) {
+	$m->delete($key);
+}
 $m->setMulti($data);
 $actual = $m->getMulti(array_keys($data));
-$cas = array();
-$actual_cas = $m->getMulti(array_keys($data), $cas);
 
 foreach ($data as $key => $value) {
-	if ($value !== $actual[$key] || $value !== $actual_cas[$key] || !isset($cas[$key])) {
-		if (isset($cas[$key]) && is_object($value)
+	if ($value !== $actual[$key]) {
+		if (is_object($value)
 			&& $value == $actual[$key]
-			&& $value == $actual_cas[$key]
-			&& get_class($value) == get_class($actual[$key])
-			&& get_class($value) == get_class($actual_cas[$key])) {
+			&& get_class($value) == get_class($actual[$key])) {
 			continue;
 		}
 		echo "=== $key ===\n";
 		echo "Expected: ";
 		var_dump($value);
 		echo "Actual: ";
 		var_dump($actual[$key]);
-		echo "Actual CAS: ";
-		var_dump($actual_cas[$key]);
-		echo "Cas: ", $cas[$key], "\n";
 	}
 }