From 1235c31867b421455a9758909144e74c954d9395 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Wed, 8 Jun 2011 12:23:02 -0700 Subject: [PATCH] Style cleanup --- libmemcached/fetch.cc | 2 +- libmemcached/get.cc | 79 +++++++++++++--------------- libmemcached/hash.cc | 2 +- libmemcached/hosts.cc | 2 +- libmemcached/watchpoint.h | 4 +- tests/mem_functions.cc | 108 +++++++++++++++++++------------------- tests/parser.cc | 20 +++++++ tests/parser.h | 3 ++ 8 files changed, 118 insertions(+), 102 deletions(-) diff --git a/libmemcached/fetch.cc b/libmemcached/fetch.cc index 71d5820b..4420ad02 100644 --- a/libmemcached/fetch.cc +++ b/libmemcached/fetch.cc @@ -56,7 +56,7 @@ char *memcached_fetch(memcached_st *ptr, char *key, size_t *key_length, result_buffer= memcached_fetch_result(ptr, result_buffer, error); - if (result_buffer == NULL || *error != MEMCACHED_SUCCESS) + if (result_buffer == NULL or *error != MEMCACHED_SUCCESS) { WATCHPOINT_ASSERT(result_buffer == NULL); *value_length= 0; diff --git a/libmemcached/get.cc b/libmemcached/get.cc index d0b01b33..576a9b7e 100644 --- a/libmemcached/get.cc +++ b/libmemcached/get.cc @@ -67,11 +67,6 @@ char *memcached_get_by_key(memcached_st *ptr, uint32_t *flags, memcached_return_t *error) { - char *value; - size_t dummy_length; - uint32_t dummy_flags; - memcached_return_t dummy_error; - unlikely (ptr->flags.use_udp) { *error= MEMCACHED_NOT_SUPPORTED; @@ -83,8 +78,8 @@ char *memcached_get_by_key(memcached_st *ptr, (const char * const *)&key, &key_length, 1, false); - value= memcached_fetch(ptr, NULL, NULL, - value_length, flags, error); + char *value= memcached_fetch(ptr, NULL, NULL, + value_length, flags, error); /* This is for historical reasons */ if (*error == MEMCACHED_END) *error= MEMCACHED_NOTFOUND; @@ -138,10 +133,15 @@ char *memcached_get_by_key(memcached_st *ptr, return NULL; } - (void)memcached_fetch(ptr, NULL, NULL, - &dummy_length, &dummy_flags, - &dummy_error); + size_t dummy_length; + uint32_t dummy_flags; + memcached_return_t dummy_error; + + char *dummy_value= memcached_fetch(ptr, NULL, NULL, + &dummy_length, &dummy_flags, + &dummy_error); WATCHPOINT_ASSERT(dummy_length == 0); + WATCHPOINT_ASSERT(dummy_value == 0); return value; } @@ -622,48 +622,43 @@ static memcached_return_t binary_mget_by_key(memcached_st *ptr, size_t number_of_keys, bool mget_mode) { - memcached_return_t rc; - if (ptr->number_of_replicas == 0) { - rc= simple_binary_mget(ptr, master_server_key, is_group_key_set, - keys, key_length, number_of_keys, mget_mode); + return simple_binary_mget(ptr, master_server_key, is_group_key_set, + keys, key_length, number_of_keys, mget_mode); } - else - { - uint32_t* hash= static_cast(libmemcached_malloc(ptr, sizeof(uint32_t) * number_of_keys)); - bool* dead_servers= static_cast(libmemcached_calloc(ptr, memcached_server_count(ptr), sizeof(bool))); - if (hash == NULL || dead_servers == NULL) - { - libmemcached_free(ptr, hash); - libmemcached_free(ptr, dead_servers); - return MEMCACHED_MEMORY_ALLOCATION_FAILURE; - } + uint32_t* hash= static_cast(libmemcached_malloc(ptr, sizeof(uint32_t) * number_of_keys)); + bool* dead_servers= static_cast(libmemcached_calloc(ptr, memcached_server_count(ptr), sizeof(bool))); - if (is_group_key_set) + if (hash == NULL || dead_servers == NULL) + { + libmemcached_free(ptr, hash); + libmemcached_free(ptr, dead_servers); + return MEMCACHED_MEMORY_ALLOCATION_FAILURE; + } + + if (is_group_key_set) + { + for (size_t x= 0; x < number_of_keys; x++) { - for (size_t x= 0; x < number_of_keys; x++) - { - hash[x]= master_server_key; - } + hash[x]= master_server_key; } - else + } + else + { + for (size_t x= 0; x < number_of_keys; x++) { - for (size_t x= 0; x < number_of_keys; x++) - { - hash[x]= memcached_generate_hash_with_redistribution(ptr, keys[x], key_length[x]); - } + hash[x]= memcached_generate_hash_with_redistribution(ptr, keys[x], key_length[x]); } + } - rc= replication_binary_mget(ptr, hash, dead_servers, keys, - key_length, number_of_keys); + memcached_return_t rc= replication_binary_mget(ptr, hash, dead_servers, keys, + key_length, number_of_keys); - libmemcached_free(ptr, hash); - libmemcached_free(ptr, dead_servers); + WATCHPOINT_IFERROR(rc); + libmemcached_free(ptr, hash); + libmemcached_free(ptr, dead_servers); - return MEMCACHED_SUCCESS; - } - - return rc; + return MEMCACHED_SUCCESS; } diff --git a/libmemcached/hash.cc b/libmemcached/hash.cc index 5eca0c6f..e239320d 100644 --- a/libmemcached/hash.cc +++ b/libmemcached/hash.cc @@ -62,7 +62,7 @@ static uint32_t dispatch_host(const memcached_st *ptr, uint32_t hash) case MEMCACHED_DISTRIBUTION_CONSISTENT_KETAMA_SPY: { uint32_t num= ptr->ketama.continuum_points_counter; - WATCHPOINT_ASSERT(ptr->continuum); + WATCHPOINT_ASSERT(ptr->ketama.continuum); hash= hash; memcached_continuum_item_st *begin, *end, *left, *right, *middle; diff --git a/libmemcached/hosts.cc b/libmemcached/hosts.cc index 2f600197..4acc3cdd 100644 --- a/libmemcached/hosts.cc +++ b/libmemcached/hosts.cc @@ -325,7 +325,7 @@ static memcached_return_t update_continuum(memcached_st *ptr) } WATCHPOINT_ASSERT(ptr); - WATCHPOINT_ASSERT(ptr->continuum); + WATCHPOINT_ASSERT(ptr->ketama.continuum); WATCHPOINT_ASSERT(memcached_server_count(ptr) * MEMCACHED_POINTS_PER_SERVER <= MEMCACHED_CONTINUUM_SIZE); ptr->ketama.continuum_points_counter= pointer_counter; qsort(ptr->ketama.continuum, ptr->ketama.continuum_points_counter, sizeof(memcached_continuum_item_st), continuum_item_cmp); diff --git a/libmemcached/watchpoint.h b/libmemcached/watchpoint.h index b4fa50e6..58875e09 100644 --- a/libmemcached/watchpoint.h +++ b/libmemcached/watchpoint.h @@ -96,14 +96,14 @@ static inline void libmemcached_stack_dump(void) #define WATCHPOINT #define WATCHPOINT_ERROR(A) -#define WATCHPOINT_IFERROR(A) +#define WATCHPOINT_IFERROR(__memcached_return_t) (void)(__memcached_return_t) #define WATCHPOINT_STRING(A) #define WATCHPOINT_NUMBER(A) #define WATCHPOINT_LABELED_NUMBER(A,B) #define WATCHPOINT_IF_LABELED_NUMBER(A,B,C) #define WATCHPOINT_ERRNO(A) #define WATCHPOINT_ASSERT_PRINT(A,B,C) -#define WATCHPOINT_ASSERT(A) +#define WATCHPOINT_ASSERT(A) (void)(A) #define WATCHPOINT_ASSERT_INITIALIZED(A) #define WATCHPOINT_SET(A) diff --git a/tests/mem_functions.cc b/tests/mem_functions.cc index 27542a47..b48f0d08 100644 --- a/tests/mem_functions.cc +++ b/tests/mem_functions.cc @@ -840,25 +840,25 @@ static test_return_t bad_key_test(memcached_st *memc) test_compare(query_id, memcached_query_id(memc_clone)); // We should not increase the query_id for memcached_behavior_set() /* All keys are valid in the binary protocol (except for length) */ - if (memcached_behavior_get(memc_clone, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) == 0) + if (not memcached_behavior_get(memc_clone, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL)) { query_id= memcached_query_id(memc_clone); string= memcached_get(memc_clone, key, strlen(key), &string_length, &flags, &rc); - test_true(rc == MEMCACHED_BAD_KEY_PROVIDED); - test_true(string_length == 0); - test_true(!string); + test_compare(MEMCACHED_BAD_KEY_PROVIDED, rc); + test_compare(0, string_length); + test_false(string); set= 0; query_id= memcached_query_id(memc_clone); rc= memcached_behavior_set(memc_clone, MEMCACHED_BEHAVIOR_VERIFY_KEY, set); test_compare(query_id, memcached_query_id(memc_clone)); // We should not increase the query_id for memcached_behavior_set() - test_true(rc == MEMCACHED_SUCCESS); + test_compare(MEMCACHED_SUCCESS, rc); string= memcached_get(memc_clone, key, strlen(key), &string_length, &flags, &rc); - test_true(rc == MEMCACHED_NOTFOUND); - test_true(string_length == 0); - test_true(!string); + test_compare_got(MEMCACHED_NOTFOUND, rc, memcached_strerror(NULL, rc)); + test_compare(0, string_length); + test_false(string); /* Test multi key for bad keys */ const char *keys[] = { "GoodKey", "Bad Key", "NotMine" }; @@ -1014,7 +1014,7 @@ static test_return_t get_test(memcached_st *memc) string= memcached_get(memc, key, strlen(key), &string_length, &flags, &rc); - test_true(rc == MEMCACHED_NOTFOUND); + test_compare_got(MEMCACHED_NOTFOUND, rc, memcached_strerror(NULL, rc)); test_false(string_length); test_false(string); @@ -1023,28 +1023,29 @@ static test_return_t get_test(memcached_st *memc) static test_return_t get_test2(memcached_st *memc) { - memcached_return_t rc; const char *key= "foo"; const char *value= "when we sanitize"; - char *string; - size_t string_length; - uint32_t flags; uint64_t query_id= memcached_query_id(memc); - rc= memcached_set(memc, key, strlen(key), - value, strlen(value), - (time_t)0, (uint32_t)0); - test_true(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED); + memcached_return_t rc= memcached_set(memc, key, strlen(key), + value, strlen(value), + (time_t)0, (uint32_t)0); + test_true(rc == MEMCACHED_SUCCESS or rc == MEMCACHED_BUFFERED); test_compare(query_id +1, memcached_query_id(memc)); query_id= memcached_query_id(memc); - string= memcached_get(memc, key, strlen(key), - &string_length, &flags, &rc); + test_true(query_id); + + uint32_t flags; + size_t string_length; + char *string= memcached_get(memc, key, strlen(key), + &string_length, &flags, &rc); test_compare(query_id +1, memcached_query_id(memc)); + test_compare_got(MEMCACHED_SUCCESS, rc, memcached_strerror(NULL, rc)); + test_compare_got(MEMCACHED_SUCCESS, memcached_last_error(memc), memcached_last_error_message(memc)); test_true(string); - test_true(rc == MEMCACHED_SUCCESS); - test_true(string_length == strlen(value)); + test_compare(strlen(value), string_length); test_memcmp(string, value, string_length); free(string); @@ -1054,18 +1055,16 @@ static test_return_t get_test2(memcached_st *memc) static test_return_t set_test2(memcached_st *memc) { - memcached_return_t rc; const char *key= "foo"; const char *value= "train in the brain"; size_t value_length= strlen(value); - unsigned int x; - for (x= 0; x < 10; x++) + for (uint32_t x= 0; x < 10; x++) { - rc= memcached_set(memc, key, strlen(key), - value, value_length, - (time_t)0, (uint32_t)0); - test_true(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED); + memcached_return_t rc= memcached_set(memc, key, strlen(key), + value, value_length, + (time_t)0, (uint32_t)0); + test_true(rc == MEMCACHED_SUCCESS or rc == MEMCACHED_BUFFERED); } return TEST_SUCCESS; @@ -1073,15 +1072,15 @@ static test_return_t set_test2(memcached_st *memc) static test_return_t set_test3(memcached_st *memc) { - memcached_return_t rc; - char *value; size_t value_length= 8191; - value = (char*)malloc(value_length); + char *value= (char*)malloc(value_length); test_true(value); for (uint32_t x= 0; x < value_length; x++) + { value[x] = (char) (x % 127); + } /* The dump test relies on there being at least 32 items in memcached */ for (uint32_t x= 0; x < 32; x++) @@ -1091,9 +1090,9 @@ static test_return_t set_test3(memcached_st *memc) snprintf(key, sizeof(key), "foo%u", x); uint64_t query_id= memcached_query_id(memc); - rc= memcached_set(memc, key, strlen(key), - value, value_length, - (time_t)0, (uint32_t)0); + memcached_return_t rc= memcached_set(memc, key, strlen(key), + value, value_length, + (time_t)0, (uint32_t)0); test_true(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED); test_compare(query_id +1, memcached_query_id(memc)); } @@ -1105,33 +1104,32 @@ static test_return_t set_test3(memcached_st *memc) static test_return_t get_test3(memcached_st *memc) { - memcached_return_t rc; const char *key= "foo"; - char *value; size_t value_length= 8191; - char *string; - size_t string_length; - uint32_t flags; - uint32_t x; - value = (char*)malloc(value_length); + char *value= (char*)malloc(value_length); test_true(value); - for (x= 0; x < value_length; x++) + for (uint32_t x= 0; x < value_length; x++) + { value[x] = (char) (x % 127); + } + memcached_return_t rc; rc= memcached_set(memc, key, strlen(key), value, value_length, (time_t)0, (uint32_t)0); - test_true(rc == MEMCACHED_SUCCESS || rc == MEMCACHED_BUFFERED); + test_true(rc == MEMCACHED_SUCCESS or rc == MEMCACHED_BUFFERED); - string= memcached_get(memc, key, strlen(key), - &string_length, &flags, &rc); + size_t string_length; + uint32_t flags; + char *string= memcached_get(memc, key, strlen(key), + &string_length, &flags, &rc); - test_true(rc == MEMCACHED_SUCCESS); + test_compare(MEMCACHED_SUCCESS, rc); test_true(string); - test_true(string_length == value_length); - test_true(!memcmp(string, value, string_length)); + test_compare(string_length, value_length); + test_memcmp(string, value, string_length); free(string); free(value); @@ -1692,19 +1690,19 @@ static test_return_t mget_test(memcached_st *memc) /* We need to empty the server before continueing test */ rc= memcached_flush(memc, 0); - test_true(rc == MEMCACHED_SUCCESS); + test_compare(MEMCACHED_SUCCESS, rc); rc= memcached_mget(memc, keys, key_length, 3); - test_true(rc == MEMCACHED_SUCCESS); + test_compare(MEMCACHED_SUCCESS, rc); while ((return_value= memcached_fetch(memc, return_key, &return_key_length, &return_value_length, &flags, &rc)) != NULL) { test_true(return_value); } - test_true(!return_value); - test_true(return_value_length == 0); - test_true(rc == MEMCACHED_END); + test_false(return_value); + test_compare(0, return_value_length); + test_compare(MEMCACHED_END, rc); for (x= 0; x < 3; x++) { @@ -3019,9 +3017,8 @@ static test_return_t user_supplied_bug18(memcached_st *trash) */ /* sighandler_t function that always asserts false */ -static void fail(int unused) +static void fail(int) { - (void)unused; assert(0); } @@ -6117,6 +6114,7 @@ test_st regression_tests[]= { {"lp:?", 1, (test_callback_fn)regression_bug_ }, {"lp:728286", 1, (test_callback_fn)regression_bug_728286 }, {"lp:581030", 1, (test_callback_fn)regression_bug_581030 }, + {"lp:71231153", 1, (test_callback_fn)regression_bug_71231153 }, {0, 0, (test_callback_fn)0} }; diff --git a/tests/parser.cc b/tests/parser.cc index 7a8b8690..e2f6b2f8 100644 --- a/tests/parser.cc +++ b/tests/parser.cc @@ -545,3 +545,23 @@ test_return_t test_hostname_port_weight(memcached_st *) return TEST_SUCCESS; } + +test_return_t regression_bug_71231153(memcached_st *) +{ + if (1) return TEST_SKIPPED; + + memcached_st *memc= memcached(memcached_literal_param("--SERVER=10.0.2.252 --CONNECT-TIMEOUT=1 --POLL-TIMEOUT=1")); + test_true(memc); + test_compare(1, memc->connect_timeout); + test_compare(1, memc->poll_timeout); + + memcached_return_t rc; + size_t value_len; + char *value= memcached_get(memc, "test", 4, &value_len, NULL, &rc); + test_false(value); + test_compare_got(MEMCACHED_TIMEOUT, rc, memcached_last_error_message(memc)); + + memcached_free(memc); + + return TEST_SUCCESS; +} diff --git a/tests/parser.h b/tests/parser.h index dba9bc59..98d3c1be 100644 --- a/tests/parser.h +++ b/tests/parser.h @@ -100,6 +100,9 @@ test_return_t server_with_weight_test(memcached_st *); LIBTEST_INTERNAL_API test_return_t test_hostname_port_weight(memcached_st *); +LIBTEST_INTERNAL_API +test_return_t regression_bug_71231153(memcached_st *); + #ifdef __cplusplus } #endif -- 2.30.2