From fa7a1cc0dd5eb55a8a436a339586ffee32ed4328 Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Mon, 5 Jan 2009 15:24:28 -0800 Subject: [PATCH] Refactor to remove leak in new framework for clone() of server. --- libmemcached/memcached.c | 8 ++++---- libmemcached/memcached.h | 5 ++++- libmemcached/memcached_connect.c | 4 ++-- libmemcached/memcached_constants.h | 7 ------- libmemcached/memcached_fetch.c | 4 +--- libmemcached/memcached_result.c | 10 ++------- libmemcached/memcached_result.h | 2 +- libmemcached/memcached_server.c | 33 +++++++----------------------- libmemcached/memcached_server.h | 4 ++-- libmemcached/memcached_string.c | 16 +++------------ libmemcached/memcached_string.h | 2 +- tests/function.c | 8 +++++--- 12 files changed, 32 insertions(+), 71 deletions(-) diff --git a/libmemcached/memcached.c b/libmemcached/memcached.c index 767075d4..1144115f 100644 --- a/libmemcached/memcached.c +++ b/libmemcached/memcached.c @@ -15,7 +15,7 @@ memcached_st *memcached_create(memcached_st *ptr) return NULL; /* MEMCACHED_MEMORY_ALLOCATION_FAILURE */ memset(ptr, 0, sizeof(memcached_st)); - ptr->is_allocated= MEMCACHED_ALLOCATED; + ptr->is_allocated= true; } else { @@ -53,7 +53,7 @@ void memcached_free(memcached_st *ptr) free(ptr->continuum); } - if (ptr->is_allocated == MEMCACHED_ALLOCATED) + if (ptr->is_allocated) { if (ptr->call_free) ptr->call_free(ptr, ptr); @@ -61,7 +61,7 @@ void memcached_free(memcached_st *ptr) free(ptr); } else - ptr->is_allocated= MEMCACHED_USED; + memset(ptr, 0, sizeof(memcached_st)); } /* @@ -77,7 +77,7 @@ memcached_st *memcached_clone(memcached_st *clone, memcached_st *source) if (source == NULL) return memcached_create(clone); - if (clone && clone->is_allocated == MEMCACHED_USED) + if (clone && clone->is_allocated) { return NULL; } diff --git a/libmemcached/memcached.h b/libmemcached/memcached.h index ebe5f239..0ebe6e30 100644 --- a/libmemcached/memcached.h +++ b/libmemcached/memcached.h @@ -12,6 +12,9 @@ #include #include +#if !defined(__cplusplus) +# include +#endif #include #include @@ -70,7 +73,7 @@ struct memcached_stat_st { struct memcached_st { uint8_t purging; - memcached_allocated is_allocated; + bool is_allocated; memcached_server_st *hosts; uint32_t number_of_hosts; uint32_t cursor_server; diff --git a/libmemcached/memcached_connect.c b/libmemcached/memcached_connect.c index 3ae0b611..c5157629 100644 --- a/libmemcached/memcached_connect.c +++ b/libmemcached/memcached_connect.c @@ -191,7 +191,7 @@ static memcached_return network_connect(memcached_server_st *ptr) } } - if (ptr->sockaddr_inited == MEMCACHED_NOT_ALLOCATED || + if (ptr->sockaddr_inited || (!(ptr->root->flags & MEM_USE_CACHE_LOOKUPS))) { memcached_return rc; @@ -199,7 +199,7 @@ static memcached_return network_connect(memcached_server_st *ptr) rc= set_hostinfo(ptr); if (rc != MEMCACHED_SUCCESS) return rc; - ptr->sockaddr_inited= MEMCACHED_ALLOCATED; + ptr->sockaddr_inited= true; } use= ptr->address_info; diff --git a/libmemcached/memcached_constants.h b/libmemcached/memcached_constants.h index 5fea0208..67a00445 100644 --- a/libmemcached/memcached_constants.h +++ b/libmemcached/memcached_constants.h @@ -131,13 +131,6 @@ typedef enum { MEMCACHED_CONNECTION_UNIX_SOCKET } memcached_connection; -typedef enum { - MEMCACHED_NOT_ALLOCATED, - MEMCACHED_ALLOCATED, - MEMCACHED_USED -} memcached_allocated; - - #ifdef __cplusplus } #endif diff --git a/libmemcached/memcached_fetch.c b/libmemcached/memcached_fetch.c index 4cd3e6a5..f728bebc 100644 --- a/libmemcached/memcached_fetch.c +++ b/libmemcached/memcached_fetch.c @@ -187,8 +187,6 @@ memcached_result_st *memcached_fetch_result(memcached_st *ptr, if (result == NULL) result= memcached_result_create(ptr, NULL); - WATCHPOINT_ASSERT(result->value.is_allocated != MEMCACHED_USED); - #ifdef UNUSED if (ptr->flags & MEM_NO_BLOCK) memcached_io_preread(ptr); @@ -219,7 +217,7 @@ memcached_result_st *memcached_fetch_result(memcached_st *ptr, } /* We have completed reading data */ - if (result->is_allocated == MEMCACHED_ALLOCATED) + if (result->is_allocated) memcached_result_free(result); else memcached_string_reset(&result->value); diff --git a/libmemcached/memcached_result.c b/libmemcached/memcached_result.c index 0860e792..8d376f68 100644 --- a/libmemcached/memcached_result.c +++ b/libmemcached/memcached_result.c @@ -11,10 +11,7 @@ memcached_result_st *memcached_result_create(memcached_st *memc, { /* Saving malloc calls :) */ if (ptr) - { memset(ptr, 0, sizeof(memcached_result_st)); - ptr->is_allocated= MEMCACHED_NOT_ALLOCATED; - } else { if (memc->call_malloc) @@ -25,13 +22,12 @@ memcached_result_st *memcached_result_create(memcached_st *memc, if (ptr == NULL) return NULL; memset(ptr, 0, sizeof(memcached_result_st)); - ptr->is_allocated= MEMCACHED_ALLOCATED; + ptr->is_allocated= true; } ptr->root= memc; memcached_string_create(memc, &ptr->value, 0); WATCHPOINT_ASSERT(ptr->value.string == NULL); - WATCHPOINT_ASSERT(ptr->value.is_allocated == MEMCACHED_NOT_ALLOCATED); return ptr; } @@ -60,8 +56,6 @@ void memcached_result_free(memcached_result_st *ptr) memcached_string_free(&ptr->value); - if (ptr->is_allocated == MEMCACHED_ALLOCATED) + if (ptr->is_allocated) free(ptr); - else - ptr->is_allocated= MEMCACHED_USED; } diff --git a/libmemcached/memcached_result.h b/libmemcached/memcached_result.h index 4826042c..69daaf5e 100644 --- a/libmemcached/memcached_result.h +++ b/libmemcached/memcached_result.h @@ -14,7 +14,7 @@ extern "C" { #endif struct memcached_result_st { - memcached_allocated is_allocated; + bool is_allocated; memcached_st *root; char key[MEMCACHED_MAX_KEY]; size_t key_length; diff --git a/libmemcached/memcached_server.c b/libmemcached/memcached_server.c index fe132fe1..75e44d0c 100644 --- a/libmemcached/memcached_server.c +++ b/libmemcached/memcached_server.c @@ -13,13 +13,10 @@ memcached_server_st *memcached_server_create(memcached_st *memc, memcached_serve return NULL; /* MEMCACHED_MEMORY_ALLOCATION_FAILURE */ memset(ptr, 0, sizeof(memcached_server_st)); - ptr->is_allocated= MEMCACHED_ALLOCATED; + ptr->is_allocated= true; } else - { - ptr->is_allocated= MEMCACHED_USED; memset(ptr, 0, sizeof(memcached_server_st)); - } ptr->root= memc; @@ -44,7 +41,6 @@ memcached_server_st *memcached_server_create_with(memcached_st *memc, memcached_ host->read_ptr= host->read_buffer; if (memc) host->next_retry= memc->retry_timeout; - host->sockaddr_inited= MEMCACHED_NOT_ALLOCATED; return host; } @@ -59,13 +55,15 @@ void memcached_server_free(memcached_server_st *ptr) ptr->address_info= NULL; } - if (ptr->is_allocated == MEMCACHED_ALLOCATED) + if (ptr->is_allocated) { if (ptr->root && ptr->root->call_free) ptr->root->call_free(ptr->root, ptr); else free(ptr); } + else + memset(ptr, 0, sizeof(memcached_server_st)); } /* @@ -73,31 +71,14 @@ void memcached_server_free(memcached_server_st *ptr) */ memcached_server_st *memcached_server_clone(memcached_server_st *clone, memcached_server_st *ptr) { - memcached_server_st *new_clone; - /* We just do a normal create if ptr is missing */ if (ptr == NULL) return NULL; - if (clone && clone->is_allocated == MEMCACHED_USED) - { - WATCHPOINT_ASSERT(0); - return NULL; - } - - new_clone= memcached_server_create(ptr->root, clone); - - if (new_clone == NULL) - return NULL; - - new_clone->root= ptr->root; - /* TODO We should check return type */ - memcached_server_create_with(new_clone->root, new_clone, - ptr->hostname, ptr->port, ptr->weight, - ptr->type); - - return new_clone; + return memcached_server_create_with(ptr->root, clone, + ptr->hostname, ptr->port, ptr->weight, + ptr->type); } memcached_return memcached_server_cursor(memcached_st *ptr, diff --git a/libmemcached/memcached_server.h b/libmemcached/memcached_server.h index ca81b0d3..8b60a384 100644 --- a/libmemcached/memcached_server.h +++ b/libmemcached/memcached_server.h @@ -14,7 +14,7 @@ extern "C" { #endif struct memcached_server_st { - memcached_allocated is_allocated; + bool is_allocated; char hostname[MEMCACHED_MAX_HOST_LENGTH]; unsigned int port; int fd; @@ -26,7 +26,7 @@ struct memcached_server_st { size_t read_data_length; size_t read_buffer_length; char *read_ptr; - memcached_allocated sockaddr_inited; + bool sockaddr_inited; struct addrinfo *address_info; memcached_connection type; uint8_t major_version; diff --git a/libmemcached/memcached_string.c b/libmemcached/memcached_string.c index 3cc894d5..5a6f190e 100644 --- a/libmemcached/memcached_string.c +++ b/libmemcached/memcached_string.c @@ -41,10 +41,7 @@ memcached_string_st *memcached_string_create(memcached_st *ptr, memcached_string /* Saving malloc calls :) */ if (string) - { memset(string, 0, sizeof(memcached_string_st)); - string->is_allocated= MEMCACHED_NOT_ALLOCATED; - } else { if (ptr->call_malloc) @@ -55,7 +52,7 @@ memcached_string_st *memcached_string_create(memcached_st *ptr, memcached_string if (string == NULL) return NULL; memset(string, 0, sizeof(memcached_string_st)); - string->is_allocated= MEMCACHED_ALLOCATED; + string->is_allocated= true; } string->block_size= MEMCACHED_BLOCK_SIZE; string->root= ptr; @@ -81,8 +78,6 @@ memcached_return memcached_string_append_character(memcached_string_st *string, { memcached_return rc; - WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); - rc= memcached_string_check(string, 1); if (rc != MEMCACHED_SUCCESS) @@ -99,8 +94,6 @@ memcached_return memcached_string_append(memcached_string_st *string, { memcached_return rc; - WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); - rc= memcached_string_check(string, length); if (rc != MEMCACHED_SUCCESS) @@ -120,8 +113,6 @@ char *memcached_string_c_copy(memcached_string_st *string) { char *c_ptr; - WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); - if (memcached_string_length(string) == 0) return NULL; @@ -141,7 +132,6 @@ char *memcached_string_c_copy(memcached_string_st *string) memcached_return memcached_string_reset(memcached_string_st *string) { - WATCHPOINT_ASSERT(string->is_allocated != MEMCACHED_USED); string->end= string->string; return MEMCACHED_SUCCESS; @@ -160,7 +150,7 @@ void memcached_string_free(memcached_string_st *ptr) free(ptr->string); } - if (ptr->is_allocated == MEMCACHED_ALLOCATED) + if (ptr->is_allocated) { if (ptr->root->call_free) ptr->root->call_free(ptr->root, ptr); @@ -168,5 +158,5 @@ void memcached_string_free(memcached_string_st *ptr) free(ptr); } else - ptr->is_allocated= MEMCACHED_USED; + memset(ptr, 0, sizeof(memcached_string_st)); } diff --git a/libmemcached/memcached_string.h b/libmemcached/memcached_string.h index 403602ed..e7e6db4d 100644 --- a/libmemcached/memcached_string.h +++ b/libmemcached/memcached_string.h @@ -15,7 +15,7 @@ extern "C" { struct memcached_string_st { memcached_st *root; - memcached_allocated is_allocated; + bool is_allocated; char *end; size_t current_size; size_t block_size; diff --git a/tests/function.c b/tests/function.c index b801b7c3..03b1da6f 100644 --- a/tests/function.c +++ b/tests/function.c @@ -2146,6 +2146,8 @@ test_return user_supplied_bug19(memcached_st *memc) memcached_server_free(s); memcached_free(m); + + return 0; } #include "ketama_test_cases.h" @@ -2190,7 +2192,7 @@ test_return user_supplied_bug18(memcached_st *memc) /* VDEAAAAA hashes to fffcd1b5, after the last continuum point, and lets * us test the boundary wraparound. */ - assert(memcached_generate_hash(memc, (unsigned char *)"VDEAAAAA", 8) == memc->continuum[0].index); + assert(memcached_generate_hash(memc, (char *)"VDEAAAAA", 8) == memc->continuum[0].index); /* verify the standard ketama set. */ for (i= 0; i < 99; i++) @@ -2208,7 +2210,7 @@ static test_return result_static(memcached_st *memc) memcached_result_st *result_ptr; result_ptr= memcached_result_create(memc, &result); - assert(result.is_allocated == MEMCACHED_NOT_ALLOCATED); + assert(result.is_allocated == false); assert(result_ptr); memcached_result_free(&result); @@ -2232,7 +2234,7 @@ static test_return string_static_null(memcached_st *memc) memcached_string_st *string_ptr; string_ptr= memcached_string_create(memc, &string, 0); - assert(string.is_allocated == MEMCACHED_NOT_ALLOCATED); + assert(string.is_allocated == false); assert(string_ptr); memcached_string_free(&string); -- 2.30.2