Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 9, 2017

Conversation

TysonAndre
Copy link
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
Contributor

@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