From: Brian Aker <brian@tangent.org>
Date: Mon, 2 May 2011 23:01:50 +0000 (-0700)
Subject: Fix issue where hash might not be quite initialized. Also remove redundant bit to... 
X-Git-Tag: 0.51~10^2~19
X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=4cd075fbd0fab78e393c1559db340a221196a006;p=m6w6%2Flibmemcached

Fix issue where hash might not be quite initialized. Also remove redundant bit to memcached_st
---

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 <libhashkit/common.h>
 
-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);