From a7ebdf0e1f2a87790c492dcc38a2944d04e05391 Mon Sep 17 00:00:00 2001 From: Aaron Stone Date: Wed, 25 Jan 2017 14:18:17 -0800 Subject: [PATCH] Support for 64-bit offsets for increment and decrement While the memcached protocol allows for 64-bit increments or decrements, there is a quirk in the libmemcached API that the memcached_increment() and memcached_decrement() functions take only a 32-bit adjustment value. Since the memcached_increment_by_key() and memcached_decrement_by_key() functions do accept 64-bit adjustment values, and the memcached_increment() and memcached_decrement() functions are simply wrappers around those, we'll use them directly and thus support 64-bit adjustments in all cases. --- php_memcached.c | 25 +++++++++++++++---------- tests/incrdecr.phpt | 22 ++++++++++++++++++---- tests/incrdecr_bykey.phpt | 22 ++++++++++++++++++---- 3 files changed, 51 insertions(+), 18 deletions(-) diff --git a/php_memcached.c b/php_memcached.c index 1e73a4f9..93d8738f 100644 --- a/php_memcached.c +++ b/php_memcached.c @@ -2185,7 +2185,7 @@ static void php_memc_deleteMulti_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by static void php_memc_incdec_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key, zend_bool incr) { zend_string *key, *server_key = NULL; - long offset = 1; + zend_long offset = 1; uint64_t value = UINT64_MAX, initial = 0; time_t expiry = 0; memcached_return status; @@ -2208,22 +2208,27 @@ static void php_memc_incdec_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key, MEMC_CHECK_KEY(intern, key); if (offset < 0) { - php_error_docref(NULL, E_WARNING, "offset has to be > 0"); + php_error_docref(NULL, E_WARNING, "offset cannot be a negative value"); RETURN_FALSE; } if ((!by_key && n_args < 3) || (by_key && n_args < 4)) { if (by_key) { if (incr) { - status = memcached_increment_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, &value); + status = memcached_increment_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), offset, &value); } else { - status = memcached_decrement_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, &value); + status = memcached_decrement_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), offset, &value); } } else { + /* The libmemcached API has a quirk that memcached_increment() takes only a 32-bit + * offset, but memcached_increment_by_key() and all other increment and decrement + * functions take a 64-bit offset. The memcached protocol allows increment/decrement + * greater than UINT_MAX, so we just work around memcached_increment() here. + */ if (incr) { - status = memcached_increment(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, &value); + status = memcached_increment_by_key(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), ZSTR_VAL(key), ZSTR_LEN(key), offset, &value); } else { - status = memcached_decrement(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, &value); + status = memcached_decrement_by_key(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), ZSTR_VAL(key), ZSTR_LEN(key), offset, &value); } } @@ -2237,15 +2242,15 @@ static void php_memc_incdec_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key, } if (by_key) { if (incr) { - status = memcached_increment_with_initial_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, initial, expiry, &value); + status = memcached_increment_with_initial_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), offset, initial, expiry, &value); } else { - status = memcached_decrement_with_initial_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, initial, expiry, &value); + status = memcached_decrement_with_initial_by_key(intern->memc, ZSTR_VAL(server_key), ZSTR_LEN(server_key), ZSTR_VAL(key), ZSTR_LEN(key), offset, initial, expiry, &value); } } else { if (incr) { - status = memcached_increment_with_initial(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, initial, expiry, &value); + status = memcached_increment_with_initial(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), offset, initial, expiry, &value); } else { - status = memcached_decrement_with_initial(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), (unsigned int)offset, initial, expiry, &value); + status = memcached_decrement_with_initial(intern->memc, ZSTR_VAL(key), ZSTR_LEN(key), offset, initial, expiry, &value); } } if (s_should_retry_write(intern, status) && retries-- > 0) { diff --git a/tests/incrdecr.phpt b/tests/incrdecr.phpt index fc640985..cb3914a4 100644 --- a/tests/incrdecr.phpt +++ b/tests/incrdecr.phpt @@ -29,14 +29,25 @@ $m->decrement('foo', 2); var_dump($m->get('foo')); error_reporting(0); -echo "Invalid offset\n"; + +echo "Negative offset\n"; +$php_errormsg = ''; $m->increment('foo', -1); echo $php_errormsg, "\n"; var_dump($m->get('foo')); + +$php_errormsg = ''; $m->decrement('foo', -1); echo $php_errormsg, "\n"; var_dump($m->get('foo')); +echo "Enormous offset\n"; +$m->increment('foo', 4294967296); +var_dump($m->get('foo')); + +$m->decrement('foo', 4294967296); +var_dump($m->get('foo')); + --EXPECT-- Not there bool(false) @@ -51,8 +62,11 @@ int(2) int(4) int(3) int(1) -Invalid offset -Memcached::increment(): offset has to be > 0 +Negative offset +Memcached::increment(): offset cannot be a negative value +int(1) +Memcached::decrement(): offset cannot be a negative value int(1) -Memcached::decrement(): offset has to be > 0 +Enormous offset +int(4294967297) int(1) diff --git a/tests/incrdecr_bykey.phpt b/tests/incrdecr_bykey.phpt index 9c2db8dd..809f3b83 100644 --- a/tests/incrdecr_bykey.phpt +++ b/tests/incrdecr_bykey.phpt @@ -26,14 +26,25 @@ $m->decrementByKey('foo', 'foo', 2); var_dump($m->get('foo')); error_reporting(0); -echo "Invalid offset\n"; + +echo "Negative offset\n"; +$php_errormsg = ''; $m->incrementByKey('foo', 'foo', -1); echo $php_errormsg, "\n"; var_dump($m->get('foo')); + +$php_errormsg = ''; $m->decrementByKey('foo', 'foo', -1); echo $php_errormsg, "\n"; var_dump($m->get('foo')); +echo "Enormous offset\n"; +$m->incrementByKey('foo', 'foo', 4294967296); +var_dump($m->get('foo')); + +$m->decrementByKey('foo', 'foo', 4294967296); +var_dump($m->get('foo')); + --EXPECT-- Not there bool(false) @@ -45,8 +56,11 @@ int(2) int(4) int(3) int(1) -Invalid offset -Memcached::incrementByKey(): offset has to be > 0 +Negative offset +Memcached::incrementByKey(): offset cannot be a negative value +int(1) +Memcached::decrementByKey(): offset cannot be a negative value int(1) -Memcached::decrementByKey(): offset has to be > 0 +Enormous offset +int(4294967297) int(1)