From: Brian Aker Date: Sun, 25 Nov 2007 08:34:33 +0000 (-0800) Subject: Cleanedup test cases (they had some valgrind warnings). X-Git-Tag: _11~9 X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=d1b81fd3181ddae3787fa7ca9c0c7f922e7ddafd;p=m6w6%2Flibmemcached Cleanedup test cases (they had some valgrind warnings). Rewrite response parsing. Refactored server list to no longer use null serer (this is the first of a series of cleanups to this code). --- diff --git a/ChangeLog b/ChangeLog index a77151a5..15316a20 100644 --- a/ChangeLog +++ b/ChangeLog @@ -4,6 +4,7 @@ value was returned. * Bug fixed in memcached_connect() which would cause servers that did not need to be enabled to be enabled (performance issue). + * Rewrote bounds checking code for get calls 0.10 Tue Nov 20 23:22:31 PST 2007 * Added append binary test. diff --git a/include/memcached.h b/include/memcached.h index 48790ce8..122b6f97 100644 --- a/include/memcached.h +++ b/include/memcached.h @@ -120,6 +120,7 @@ struct memcached_server_st { uint8_t major_version; uint8_t minor_version; uint8_t micro_version; + uint16_t count; }; struct memcached_stat_st { diff --git a/lib/memcached_get.c b/lib/memcached_get.c index c5156439..98e24d9f 100644 --- a/lib/memcached_get.c +++ b/lib/memcached_get.c @@ -34,7 +34,7 @@ static memcached_return memcached_value_fetch(memcached_st *ptr, char *key, size { *key_length= 0; - for (; end_ptr > string_ptr && *string_ptr != ' '; string_ptr++) + for (; isalnum(*string_ptr); string_ptr++) { *key= *string_ptr; key++; @@ -42,7 +42,7 @@ static memcached_return memcached_value_fetch(memcached_st *ptr, char *key, size } } else /* Skip characters */ - for (; end_ptr > string_ptr && *string_ptr != ' '; string_ptr++); + for (; isalnum(*string_ptr); string_ptr++); if (end_ptr == string_ptr) goto read_error; @@ -51,7 +51,7 @@ static memcached_return memcached_value_fetch(memcached_st *ptr, char *key, size string_ptr++; if (end_ptr == string_ptr) goto read_error; - for (next_ptr= string_ptr; end_ptr > string_ptr && *string_ptr != ' '; string_ptr++); + for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++); *flags= (uint16_t)strtol(next_ptr, &string_ptr, 10); if (end_ptr == string_ptr) @@ -62,7 +62,7 @@ static memcached_return memcached_value_fetch(memcached_st *ptr, char *key, size if (end_ptr == string_ptr) goto read_error; - for (next_ptr= string_ptr; end_ptr > string_ptr && *string_ptr != ' '; string_ptr++); + for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++); value_length= (size_t)strtoll(next_ptr, &string_ptr, 10); if (end_ptr == string_ptr) @@ -77,7 +77,7 @@ static memcached_return memcached_value_fetch(memcached_st *ptr, char *key, size else { string_ptr++; - for (next_ptr= string_ptr; end_ptr > string_ptr && *string_ptr != ' '; string_ptr++); + for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++); if (cas) *cas= (size_t)strtoll(next_ptr, &string_ptr, 10); } @@ -209,15 +209,14 @@ char *memcached_get(memcached_st *ptr, char *key, size_t key_length, LIBMEMCACHED_MEMCACHED_GET_END(); - - return memcached_string_c_copy(result_buffer); + return memcached_string_c_copy(result_buffer); error: *value_length= 0; LIBMEMCACHED_MEMCACHED_GET_END(); - return NULL; + return NULL; } memcached_return memcached_mget(memcached_st *ptr, @@ -382,16 +381,16 @@ memcached_result_st *memcached_fetch_result(memcached_st *ptr, } else if (*error == MEMCACHED_END && memcached_string_length((memcached_string_st *)(&result->value)) == 0) { - goto error; + break; } else if (*error == MEMCACHED_END) { WATCHPOINT_ASSERT(0); /* If this happens we have somehow messed up the fetch */ - goto error; + break; } else if (*error != MEMCACHED_SUCCESS) { - goto error; + break; } else { @@ -399,8 +398,11 @@ memcached_result_st *memcached_fetch_result(memcached_st *ptr, } } -error: - memcached_string_reset(&result->value); + /* An error has occurred */ + if (result->is_allocated == MEMCACHED_ALLOCATED) + memcached_result_free(result); + else + memcached_string_reset(&result->value); return NULL; } diff --git a/lib/memcached_hosts.c b/lib/memcached_hosts.c index bd9e29c7..fac03707 100644 --- a/lib/memcached_hosts.c +++ b/lib/memcached_hosts.c @@ -6,11 +6,11 @@ static memcached_return server_add(memcached_st *ptr, char *hostname, unsigned int port, memcached_connection type); -static void host_reset(memcached_server_st *host, char *new_hostname, unsigned int port, +static void host_reset(memcached_server_st *host, char *hostname, unsigned int port, memcached_connection type) { memset(host, 0, sizeof(memcached_server_st)); - host->hostname= new_hostname; + host->hostname= strdup(hostname); host->port= port; host->fd= -1; host->type= type; @@ -22,35 +22,31 @@ static void host_reset(memcached_server_st *host, char *new_hostname, unsigned i memcached_return memcached_server_push(memcached_st *ptr, memcached_server_st *list) { unsigned int x; - unsigned int count; + uint16_t count; memcached_server_st *new_host_list; if (!list) return MEMCACHED_SUCCESS; - for (count= 0; list[count].hostname; count++); + count= list[0].count; new_host_list= (memcached_server_st *)realloc(ptr->hosts, - sizeof(memcached_server_st) * (count + ptr->number_of_hosts + 1)); + sizeof(memcached_server_st) * (count + ptr->number_of_hosts)); if (!new_host_list) return MEMCACHED_MEMORY_ALLOCATION_FAILURE; ptr->hosts= new_host_list; - for (x= 0; list[x].hostname; x++) + for (x= 0; x < count; x++) { - ptr->hosts[ptr->number_of_hosts].hostname= strdup(list[x].hostname); - ptr->hosts[ptr->number_of_hosts].port= list[x].port; - ptr->hosts[ptr->number_of_hosts].fd= list[x].fd; - ptr->hosts[ptr->number_of_hosts].stack_responses= list[x].stack_responses; - ptr->hosts[ptr->number_of_hosts].cursor_active= list[x].cursor_active; - ptr->hosts[ptr->number_of_hosts].type= list[x].type; + WATCHPOINT_ASSERT(list[x].hostname); + host_reset(&ptr->hosts[ptr->number_of_hosts], list[x].hostname, + list[x].port, list[x].type); ptr->number_of_hosts++; } - host_reset(&ptr->hosts[ptr->number_of_hosts], NULL, 0, - MEMCACHED_CONNECTION_UNKNOWN); + ptr->hosts[0].count= ptr->number_of_hosts; return MEMCACHED_SUCCESS; } @@ -94,41 +90,19 @@ static memcached_return server_add(memcached_st *ptr, char *hostname, memcached_connection type) { memcached_server_st *new_host_list; - char *new_hostname; LIBMEMCACHED_MEMCACHED_SERVER_ADD_START(); - if (ptr->number_of_hosts) - { - new_host_list= (memcached_server_st *)realloc(ptr->hosts, - sizeof(memcached_server_st) * (ptr->number_of_hosts+1)); - if (!new_host_list) - return MEMCACHED_MEMORY_ALLOCATION_FAILURE; - host_reset(&new_host_list[ptr->number_of_hosts], NULL, 0, - MEMCACHED_CONNECTION_UNKNOWN); - } - else - { - new_host_list= - (memcached_server_st *)malloc(sizeof(memcached_server_st) * 2); - if (!new_host_list) - return MEMCACHED_MEMORY_ALLOCATION_FAILURE; - host_reset(&new_host_list[0], NULL, 0, MEMCACHED_CONNECTION_UNKNOWN); - host_reset(&new_host_list[1], NULL, 0, MEMCACHED_CONNECTION_UNKNOWN); - } + new_host_list= (memcached_server_st *)realloc(ptr->hosts, + sizeof(memcached_server_st) * (ptr->number_of_hosts+1)); + if (!new_host_list) + return MEMCACHED_MEMORY_ALLOCATION_FAILURE; ptr->hosts= new_host_list; - new_hostname= - (char *)malloc(sizeof(char) * (strlen(hostname)+1)); - - if (!new_hostname) - return MEMCACHED_MEMORY_ALLOCATION_FAILURE; - - memcpy(new_hostname, hostname, strlen(hostname)); - new_hostname[strlen(hostname)]= 0; - host_reset(&ptr->hosts[ptr->number_of_hosts], new_hostname, port, type); + host_reset(&ptr->hosts[ptr->number_of_hosts], hostname, port, type); ptr->number_of_hosts++; + ptr->hosts[0].count++; LIBMEMCACHED_MEMCACHED_SERVER_ADD_END(); @@ -141,7 +115,6 @@ memcached_server_st *memcached_server_list_append(memcached_server_st *ptr, { unsigned int count; memcached_server_st *new_host_list; - char *new_hostname; if (!hostname) return ptr; @@ -149,55 +122,45 @@ memcached_server_st *memcached_server_list_append(memcached_server_st *ptr, if (!port) port= MEMCACHED_DEFAULT_PORT; - /* Always count so that we keep a free host at the end */ - if (ptr) + /* Increment count for hosts */ + count= 1; + if (ptr != NULL) { - for (count= 0; ptr[count].hostname; count++); - count+= 2; - new_host_list= (memcached_server_st *)realloc(ptr, sizeof(memcached_server_st) * count); - if (!new_host_list) - goto error; - host_reset(&new_host_list[count-1], NULL, 0, MEMCACHED_CONNECTION_UNKNOWN); - } - else + count+= ptr[0].count; + } + + new_host_list= (memcached_server_st *)realloc(ptr, sizeof(memcached_server_st) * count); + if (!new_host_list) { - count= 2; - new_host_list= (memcached_server_st *)malloc(sizeof(memcached_server_st) * count); - if (!new_host_list) - goto error; - host_reset(&new_host_list[0], NULL, 0, MEMCACHED_CONNECTION_UNKNOWN); - host_reset(&new_host_list[1], NULL, 0, MEMCACHED_CONNECTION_UNKNOWN); + *error= MEMCACHED_MEMORY_ALLOCATION_FAILURE; + return NULL; } - new_hostname= strdup(hostname); - - if (!new_hostname) - goto error; + host_reset(&new_host_list[count-1], hostname, port, MEMCACHED_CONNECTION_TCP); + new_host_list[0].count++; - host_reset(&new_host_list[count-2], new_hostname, port, MEMCACHED_CONNECTION_TCP); *error= MEMCACHED_SUCCESS; return new_host_list; -error: - *error= MEMCACHED_MEMORY_ALLOCATION_FAILURE; - - return NULL; } unsigned int memcached_server_list_count(memcached_server_st *ptr) { - unsigned int x; - for (x= 0; ptr[x].hostname; x++); - - return x; + if (ptr == NULL) + return 0; + + return ptr[0].count; } void memcached_server_list_free(memcached_server_st *ptr) { unsigned int x; - for (x= 0; ptr[x].hostname; x++) + if (ptr == NULL) + return; + + for (x= 0; x < ptr[0].count; x++) free(ptr[x].hostname); free(ptr); diff --git a/lib/memcached_response.c b/lib/memcached_response.c index 0755ae0c..78b314f4 100644 --- a/lib/memcached_response.c +++ b/lib/memcached_response.c @@ -44,6 +44,8 @@ memcached_return memcached_response(memcached_st *ptr, total_length++; WATCHPOINT_ASSERT(total_length < buffer_length); } + buffer_ptr++; + *buffer_ptr= 0; if (memcached_server_response_count(ptr, server_key)) memcached_server_response_decrement(ptr, server_key); diff --git a/tests/function.c b/tests/function.c index 3e3b4af4..3819eae7 100644 --- a/tests/function.c +++ b/tests/function.c @@ -148,6 +148,7 @@ uint8_t append_test(memcached_st *memc) assert(!memcmp(value, "we the people", strlen("we the people"))); assert(strlen("we the people") == value_length); assert(rc == MEMCACHED_SUCCESS); + free(value); return 0; } @@ -188,11 +189,12 @@ uint8_t append_binary_test(memcached_st *memc) store_ptr= (unsigned int *)value; x= 0; - while (*store_ptr) + while ((size_t)store_ptr < (size_t)(value + value_length)) { assert(*store_ptr == store_list[x++]); store_ptr++; } + free(value); return 0; } @@ -311,6 +313,7 @@ uint8_t prepend_test(memcached_st *memc) assert(!memcmp(value, "we the people", strlen("we the people"))); assert(strlen("we the people") == value_length); assert(rc == MEMCACHED_SUCCESS); + free(value); return 0; } @@ -1412,6 +1415,13 @@ uint8_t string_alloc_append_toobig(memcached_st *memc) return 0; } +uint8_t cleanup_pairs(memcached_st *memc) +{ + pairs_free(global_pairs); + + return 0; +} + uint8_t generate_data(memcached_st *memc) { unsigned long long x; @@ -1765,6 +1775,7 @@ test_st generate_tests[] ={ {"get_read", 0, get_read }, {"mget_read", 0, mget_read }, {"mget_read_result", 0, mget_read_result }, + {"cleanup", 0, cleanup_pairs }, {0, 0, 0} };