From 4cd075fbd0fab78e393c1559db340a221196a006 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Mon, 2 May 2011 16:01:50 -0700 Subject: [PATCH] Fix issue where hash might not be quite initialized. Also remove redundant bit to memcached_st --- libhashkit/hashkit.cc | 35 ++++++++++++++++++----------------- libhashkit/hashkit.h | 4 ++++ libmemcached/behavior.cc | 6 +++--- libmemcached/hosts.cc | 4 ++-- libmemcached/memcached.cc | 19 ++----------------- libmemcached/memcached.h | 1 - tests/mem_functions.c | 1 - 7 files changed, 29 insertions(+), 41 deletions(-) diff --git a/libhashkit/hashkit.cc b/libhashkit/hashkit.cc index 201a6dff..7f3514d1 100644 --- a/libhashkit/hashkit.cc +++ b/libhashkit/hashkit.cc @@ -8,22 +8,23 @@ #include -static inline bool _hashkit_init(hashkit_st *self) +static inline void _hashkit_init(hashkit_st *self) { self->base_hash.function= hashkit_one_at_a_time; self->base_hash.context= NULL; - self->distribution_hash.function= self->base_hash.function; - self->flags.is_base_same_distributed= false; - return true; + self->distribution_hash.function= hashkit_one_at_a_time; + self->distribution_hash.context= NULL; + + self->flags.is_base_same_distributed= true; } static inline hashkit_st *_hashkit_create(hashkit_st *self) { - if (self == NULL) + if (not self) { - self= (hashkit_st *)malloc(sizeof(hashkit_st)); - if (self == NULL) + self= new hashkit_st; + if (not self) { return NULL; } @@ -41,13 +42,10 @@ static inline hashkit_st *_hashkit_create(hashkit_st *self) hashkit_st *hashkit_create(hashkit_st *self) { self= _hashkit_create(self); - if (! self) + if (not self) return self; - if (! _hashkit_init(self)) - { - hashkit_free(self); - } + _hashkit_init(self); return self; } @@ -57,7 +55,7 @@ void hashkit_free(hashkit_st *self) { if (hashkit_is_allocated(self)) { - free(self); + delete self; } } @@ -86,10 +84,13 @@ hashkit_st *hashkit_clone(hashkit_st *destination, const hashkit_st *source) bool hashkit_compare(const hashkit_st *first, const hashkit_st *second) { - if (first->base_hash.function == second->base_hash.function && - first->base_hash.context == second->base_hash.context && - first->distribution_hash.function == second->distribution_hash.function && - first->distribution_hash.context == second->distribution_hash.context && + if (not first or not second) + return false; + + if (first->base_hash.function == second->base_hash.function and + first->base_hash.context == second->base_hash.context and + first->distribution_hash.function == second->distribution_hash.function and + first->distribution_hash.context == second->distribution_hash.context and first->flags.is_base_same_distributed == second->flags.is_base_same_distributed) { return true; diff --git a/libhashkit/hashkit.h b/libhashkit/hashkit.h index 9b8761f1..d4e9d5ea 100644 --- a/libhashkit/hashkit.h +++ b/libhashkit/hashkit.h @@ -90,6 +90,10 @@ void hashkit_free(hashkit_st *hash); #define hashkit_is_allocated(__object) ((__object)->options.is_allocated) #define hashkit_is_initialized(__object) ((__object)->options.is_initialized) + +#define hashkit_success(X) (X) == HASHKIT_SUCCESS +#define hashkit_failed(X) (X) != HASHKIT_SUCCESS + #ifdef __cplusplus } // extern "C" #endif diff --git a/libmemcached/behavior.cc b/libmemcached/behavior.cc index 91e33a0b..a6a2efde 100644 --- a/libmemcached/behavior.cc +++ b/libmemcached/behavior.cc @@ -239,7 +239,7 @@ uint64_t memcached_behavior_get(memcached_st *ptr, case MEMCACHED_BEHAVIOR_HASH: return hashkit_get_function(&ptr->hashkit); case MEMCACHED_BEHAVIOR_KETAMA_HASH: - return hashkit_get_function(&ptr->distribution_hashkit); + return hashkit_get_function(&ptr->hashkit); case MEMCACHED_BEHAVIOR_REMOVE_FAILED_SERVERS: case MEMCACHED_BEHAVIOR_SERVER_FAILURE_LIMIT: return ptr->server_failure_limit; @@ -403,7 +403,7 @@ memcached_hash_t memcached_behavior_get_key_hash(memcached_st *ptr) memcached_return_t memcached_behavior_set_distribution_hash(memcached_st *ptr, memcached_hash_t type) { - if (hashkit_set_function(&ptr->distribution_hashkit, (hashkit_hash_algorithm_t)type) == HASHKIT_SUCCESS) + if (hashkit_success(hashkit_set_distribution_function(&ptr->hashkit, (hashkit_hash_algorithm_t)type))) return MEMCACHED_SUCCESS; return memcached_set_error_string(ptr, MEMCACHED_INVALID_ARGUMENTS, @@ -412,7 +412,7 @@ memcached_return_t memcached_behavior_set_distribution_hash(memcached_st *ptr, m memcached_hash_t memcached_behavior_get_distribution_hash(memcached_st *ptr) { - return (memcached_hash_t)hashkit_get_function(&ptr->distribution_hashkit); + return (memcached_hash_t)hashkit_get_function(&ptr->hashkit); } const char *libmemcached_string_behavior(const memcached_behavior_t flag) diff --git a/libmemcached/hosts.cc b/libmemcached/hosts.cc index 14b45589..7527cfbd 100644 --- a/libmemcached/hosts.cc +++ b/libmemcached/hosts.cc @@ -232,7 +232,7 @@ static memcached_return_t update_continuum(memcached_st *ptr) } else { - uint32_t value= hashkit_digest(&ptr->distribution_hashkit, sort_host, (size_t)sort_host_length); + uint32_t value= hashkit_digest(&ptr->hashkit, sort_host, (size_t)sort_host_length); ptr->ketama.continuum[continuum_index].index= host_index; ptr->ketama.continuum[continuum_index++].value= value; } @@ -281,7 +281,7 @@ static memcached_return_t update_continuum(memcached_st *ptr) } else { - uint32_t value= hashkit_digest(&ptr->distribution_hashkit, sort_host, (size_t)sort_host_length); + uint32_t value= hashkit_digest(&ptr->hashkit, sort_host, (size_t)sort_host_length); ptr->ketama.continuum[continuum_index].index= host_index; ptr->ketama.continuum[continuum_index++].value= value; } diff --git a/libmemcached/memcached.cc b/libmemcached/memcached.cc index 1197dd5d..820efa50 100644 --- a/libmemcached/memcached.cc +++ b/libmemcached/memcached.cc @@ -87,9 +87,7 @@ static inline bool _memcached_init(memcached_st *self) self->distribution= MEMCACHED_DISTRIBUTION_MODULA; - hashkit_st *hash_ptr; - hash_ptr= hashkit_create(&self->hashkit); - if (! hash_ptr) + if (not hashkit_create(&self->hashkit)) return false; self->ketama.continuum= NULL; @@ -123,9 +121,6 @@ static inline bool _memcached_init(memcached_st *self) self->user_data= NULL; self->number_of_replicas= 0; - hash_ptr= hashkit_create(&self->distribution_hashkit); - if (! hash_ptr) - return false; self->allocators= memcached_allocators_return_default(); @@ -353,17 +348,7 @@ memcached_st *memcached_clone(memcached_st *clone, const memcached_st *source) new_clone->retry_timeout= source->retry_timeout; new_clone->distribution= source->distribution; - hashkit_st *hash_ptr; - - hash_ptr= hashkit_clone(&new_clone->hashkit, &source->hashkit); - if (! hash_ptr) - { - memcached_free(new_clone); - return NULL; - } - - hash_ptr= hashkit_clone(&new_clone->distribution_hashkit, &source->distribution_hashkit); - if (! hash_ptr) + if (not hashkit_clone(&new_clone->hashkit, &source->hashkit)) { memcached_free(new_clone); return NULL; diff --git a/libmemcached/memcached.h b/libmemcached/memcached.h index f32fe0e7..099a4b30 100644 --- a/libmemcached/memcached.h +++ b/libmemcached/memcached.h @@ -130,7 +130,6 @@ struct memcached_st { void *user_data; uint64_t query_id; uint32_t number_of_replicas; - hashkit_st distribution_hashkit; memcached_result_st result; struct { diff --git a/tests/mem_functions.c b/tests/mem_functions.c index 0400ce5a..91cb2b23 100644 --- a/tests/mem_functions.c +++ b/tests/mem_functions.c @@ -337,7 +337,6 @@ static test_return_t clone_test(memcached_st *memc) } test_true(memc_clone->get_key_failure == memc->get_key_failure); test_true(hashkit_compare(&memc_clone->hashkit, &memc->hashkit)); - test_true(hashkit_compare(&memc_clone->distribution_hashkit, &memc->distribution_hashkit)); test_true(memc_clone->io_bytes_watermark == memc->io_bytes_watermark); test_true(memc_clone->io_msg_watermark == memc->io_msg_watermark); test_true(memc_clone->io_key_prefetch == memc->io_key_prefetch); -- 2.30.2