Skip to content

Fix ReflectionMethod signatures for appendByKey,prepend, etc.#352

Merged
sodabrew merged 1 commit intophp-memcached-dev:masterfrom
TysonAndre:fix-signatures.php
Jun 9, 2017
Merged

Fix ReflectionMethod signatures for appendByKey,prepend, etc.#352
sodabrew merged 1 commit intophp-memcached-dev:masterfrom
TysonAndre:fix-signatures.php

Conversation

@TysonAndre
Copy link
Copy Markdown
Contributor

memc_store_impl suggests that the prepend family expects exactly 3 arguments for prependByKey(), 2 for prepend(). memcached-api.php already has the correct counts (3 for byKey, 2 for plain)

1912     if (by_key) {
1913         if (op == MEMC_OP_APPEND || op == MEMC_OP_PREPEND) {
1914             if (zend_parse_parameters(ZEND_NUM_ARGS(), "SSS", &server_key, &key, &s_value) == FAILURE) {
1915                 return;
1916             }
....
1928     } else {
1929         if (op == MEMC_OP_APPEND || op == MEMC_OP_PREPEND) {
1930             if (zend_parse_parameters(ZEND_NUM_ARGS(), "SS", &key, &s_value) == FAILURE) {

Example snippet showing the bug:

(new ReflectionMethod('Memcached', 'prepend'))->getNumberOfParameters() was wrong as a result of this. It should be 2, but was 3.

Copy link
Copy Markdown
Collaborator

@sodabrew sodabrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right - and the memcached protocol explicitly states that expiration is ignored for append/pretend if it is given.

@sodabrew sodabrew merged commit 23cf6c6 into php-memcached-dev:master Jun 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants