From 0b440c1273944e5ad216f1d6fe9e881e66ea0550 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Thu, 10 Feb 2011 15:17:29 -0800 Subject: [PATCH] Version will now have a default we can use to look for invalid versions. Also fixed regression test so that we check for invalid argment return. --- libmemcached/quit.c | 4 ++++ libmemcached/server.c | 6 +++--- libmemcached/server.h | 6 +++--- libmemcached/version.c | 41 ++++++++++++++++++++++++++++++++++++----- tests/mem_functions.c | 15 ++++++--------- tests/test.h | 16 +++++++++++++--- 6 files changed, 65 insertions(+), 23 deletions(-) diff --git a/libmemcached/quit.c b/libmemcached/quit.c index 18a9a316..85eb3015 100644 --- a/libmemcached/quit.c +++ b/libmemcached/quit.c @@ -71,6 +71,10 @@ void memcached_quit_server(memcached_server_st *ptr, bool io_death) ptr->options.is_shutting_down= false; memcached_server_response_reset(ptr); + // We reset the version so that if we end up talking to a different server + // we don't have stale server version information. + ptr->major_version= ptr->minor_version= ptr->micro_version= UINT8_MAX; + if (io_death) { ptr->server_failure_counter++; diff --git a/libmemcached/server.c b/libmemcached/server.c index 3e5e77a5..22bd6f54 100644 --- a/libmemcached/server.c +++ b/libmemcached/server.c @@ -32,9 +32,9 @@ static inline void _server_init(memcached_server_st *self, const memcached_st *r self->state.is_dead= false; WATCHPOINT_SET(self->io_wait_count.read= 0); WATCHPOINT_SET(self->io_wait_count.write= 0); - self->major_version= 0; - self->micro_version= 0; - self->minor_version= 0; + self->major_version= UINT8_MAX; + self->micro_version= UINT8_MAX; + self->minor_version= UINT8_MAX; self->type= type; self->read_ptr= self->read_buffer; self->cached_server_error= NULL; diff --git a/libmemcached/server.h b/libmemcached/server.h index 2206c7b8..4248e143 100644 --- a/libmemcached/server.h +++ b/libmemcached/server.h @@ -36,9 +36,9 @@ struct memcached_server_st { uint32_t read; uint32_t write; } io_wait_count; - uint8_t major_version; - uint8_t micro_version; - uint8_t minor_version; + uint8_t major_version; // Default definition of UINT8_MAX means that it has not been set. + uint8_t micro_version; // ditto + uint8_t minor_version; // ditto memcached_connection_t type; char *read_ptr; char *cached_server_error; diff --git a/libmemcached/version.c b/libmemcached/version.c index dde59066..8fcc11a0 100644 --- a/libmemcached/version.c +++ b/libmemcached/version.c @@ -45,10 +45,14 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr) memcached_server_write_instance_st instance= memcached_server_instance_fetch(ptr, x); + // Optimization, we only fetch version once. + if (instance->major_version != UINT8_MAX) + continue; + rrc= memcached_do(instance, command, send_length, true); if (rrc != MEMCACHED_SUCCESS) { - instance->major_version= instance->minor_version= instance->micro_version= 0; + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; rc= MEMCACHED_SOME_ERRORS; continue; } @@ -56,7 +60,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr) rrc= memcached_response(instance, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, NULL); if (rrc != MEMCACHED_SUCCESS) { - instance->major_version= instance->minor_version= instance->micro_version= 0; + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; rc= MEMCACHED_SOME_ERRORS; continue; } @@ -68,7 +72,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr) instance->major_version= (uint8_t)strtol(response_ptr, (char **)NULL, 10); if (errno == ERANGE) { - instance->major_version= instance->minor_version= instance->micro_version= 0; + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; rc= MEMCACHED_SOME_ERRORS; continue; } @@ -79,7 +83,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr) instance->minor_version= (uint8_t)strtol(response_ptr, (char **)NULL, 10); if (errno == ERANGE) { - instance->major_version= instance->minor_version= instance->micro_version= 0; + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; rc= MEMCACHED_SOME_ERRORS; continue; } @@ -89,7 +93,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr) instance->micro_version= (uint8_t)strtol(response_ptr, (char **)NULL, 10); if (errno == ERANGE) { - instance->major_version= instance->minor_version= instance->micro_version= 0; + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; rc= MEMCACHED_SOME_ERRORS; continue; } @@ -114,6 +118,9 @@ static inline memcached_return_t memcached_version_binary(memcached_st *ptr) memcached_server_write_instance_st instance= memcached_server_instance_fetch(ptr, x); + if (instance->major_version != UINT8_MAX) + continue; + rrc= memcached_do(instance, request.bytes, sizeof(request.bytes), true); if (rrc != MEMCACHED_SUCCESS) { @@ -128,6 +135,9 @@ static inline memcached_return_t memcached_version_binary(memcached_st *ptr) memcached_server_write_instance_st instance= memcached_server_instance_fetch(ptr, x); + if (instance->major_version != UINT8_MAX) + continue; + if (memcached_server_response_count(instance) > 0) { memcached_return_t rrc; @@ -143,8 +153,29 @@ static inline memcached_return_t memcached_version_binary(memcached_st *ptr) } instance->major_version= (uint8_t)strtol(buffer, &p, 10); + if (errno == ERANGE) + { + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; + rc= MEMCACHED_SOME_ERRORS; + continue; + } + instance->minor_version= (uint8_t)strtol(p + 1, &p, 10); + if (errno == ERANGE) + { + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; + rc= MEMCACHED_SOME_ERRORS; + continue; + } + instance->micro_version= (uint8_t)strtol(p + 1, NULL, 10); + if (errno == ERANGE) + { + instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX; + rc= MEMCACHED_SOME_ERRORS; + continue; + } + } } diff --git a/tests/mem_functions.c b/tests/mem_functions.c index 27cac75e..02df660d 100644 --- a/tests/mem_functions.c +++ b/tests/mem_functions.c @@ -4440,11 +4440,8 @@ static test_return_t util_version_test(memcached_st *memc) // if (! if_successful) { fprintf(stderr, "\n----------------------------------------------------------------------\n"); - fprintf(stderr, "\nDumping Server Information\n"); + fprintf(stderr, "\nDumping Server Information\n\n"); memcached_server_fn callbacks[1]; - memcached_version(memc); - - memcached_version(memc); callbacks[0]= dump_server_information; memcached_server_cursor(memc, callbacks, (void *)stderr, 1); @@ -5613,19 +5610,19 @@ static test_return_t regression_bug_463297(memcached_st *memc) memcached_return_t rc= memcached_delete(memc, "foo", 3, 1); /* but there is a bug in some of the memcached servers (1.4) that treats - * the counter as noreply so it doesn't send the proper error message - */ - test_true(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR); + * the counter as noreply so it doesn't send the proper error message + */ + test_true_got(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR || rc == MEMCACHED_INVALID_ARGUMENTS, memcached_strerror(NULL, rc)); /* And buffered mode should be disabled and we should get protocol error */ test_true(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BUFFER_REQUESTS, 1) == MEMCACHED_SUCCESS); rc= memcached_delete(memc, "foo", 3, 1); - test_true(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR); + test_true_got(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR || rc == MEMCACHED_INVALID_ARGUMENTS, memcached_strerror(NULL, rc)); /* Same goes for noreply... */ test_true(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_NOREPLY, 1) == MEMCACHED_SUCCESS); rc= memcached_delete(memc, "foo", 3, 1); - test_true(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR); + test_true_got(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR || rc == MEMCACHED_INVALID_ARGUMENTS, memcached_strerror(NULL, rc)); /* but a normal request should go through (and be buffered) */ test_true((rc= memcached_delete(memc, "foo", 3, 0)) == MEMCACHED_BUFFERED); diff --git a/tests/test.h b/tests/test.h index 77aa5303..cc0fd78a 100644 --- a/tests/test.h +++ b/tests/test.h @@ -153,7 +153,7 @@ const char *test_strerror(test_return_t code); do \ { \ if (1) { \ - fprintf(stderr, "\nFailed in %s:%d: %s\n", __FILE__, __LINE__, #A);\ + fprintf(stderr, "\nFailed at %s:%d: %s\n", __FILE__, __LINE__, #A);\ create_core(); \ return TEST_FAILURE; \ } \ @@ -163,7 +163,17 @@ do \ do \ { \ if (! (A)) { \ - fprintf(stderr, "\nAssertion failed in %s:%d: %s\n", __FILE__, __LINE__, #A);\ + fprintf(stderr, "\nAssertion failed at %s:%d: %s\n", __FILE__, __LINE__, #A);\ + create_core(); \ + return TEST_FAILURE; \ + } \ +} while (0) + +#define test_true_got(A,B) \ +do \ +{ \ + if (! (A)) { \ + fprintf(stderr, "\nAssertion failed at %s:%d: \"%s\" received \"%s\"\n", __FILE__, __LINE__, #A, (B));\ create_core(); \ return TEST_FAILURE; \ } \ @@ -173,7 +183,7 @@ do \ do \ { \ if ((A)) { \ - fprintf(stderr, "\nAssertion failed in %s:%d: %s\n", __FILE__, __LINE__, #A);\ + fprintf(stderr, "\nAssertion failed at %s:%d: %s\n", __FILE__, __LINE__, #A);\ create_core(); \ return TEST_FAILURE; \ } \ -- 2.30.2