From 32767cce940f7bcde4633cc3cd23efc28ad954bd Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Wed, 13 Jul 2011 16:27:52 -0700 Subject: [PATCH] Standardize the snprintf() failure messages, and add tests for libmemcached_util_getpid() --- configure.ac | 6 ++--- libmemcached/common.h | 4 +++ libmemcached/connect.cc | 2 ++ libmemcached/delete.cc | 3 ++- libmemcached/do.cc | 5 ++-- libmemcached/dump.cc | 3 ++- libmemcached/error.cc | 3 +++ libmemcached/flush.cc | 3 ++- libmemcached/hosts.cc | 6 +++-- libmemcached/stats.cc | 56 +++++++++++++++++++++++---------------- libmemcached/stats.h | 2 +- libmemcached/storage.cc | 6 +++-- libmemcached/util/pid.cc | 15 ++++++----- libmemcached/util/ping.cc | 1 + libmemcached/verbosity.cc | 5 +++- libtest/memcached.cc | 8 ------ libtest/server.cc | 8 ++++++ tests/mem_functions.cc | 47 +++++++++++++++++++++++++++++--- 18 files changed, 127 insertions(+), 56 deletions(-) diff --git a/configure.ac b/configure.ac index 3631b9cd..c1498462 100644 --- a/configure.ac +++ b/configure.ac @@ -8,7 +8,7 @@ # the COPYING file in this directory for full text. AC_PREREQ(2.59) -AC_INIT([libmemcached],[0.50],[http://libmemcached.org/]) +AC_INIT([libmemcached],[0.51],[http://libmemcached.org/]) AC_CONFIG_SRCDIR([libmemcached/memcached.cc]) AC_CONFIG_AUX_DIR(config) @@ -17,9 +17,9 @@ AC_CHECK_PROGS([YACC], ['bison'], [:]) AC_CHECK_PROGS([LEX], ['flex'], [:]) #shared library versioning -MEMCACHED_UTIL_LIBRARY_VERSION=2:0:0 +MEMCACHED_UTIL_LIBRARY_VERSION=2:1:1 MEMCACHED_PROTOCAL_LIBRARY_VERSION=0:0:0 -MEMCACHED_LIBRARY_VERSION=8:0:0 +MEMCACHED_LIBRARY_VERSION=8:1:1 # | | | # +------+ | +---+ # | | | diff --git a/libmemcached/common.h b/libmemcached/common.h index 46230ce3..f30589ca 100644 --- a/libmemcached/common.h +++ b/libmemcached/common.h @@ -102,6 +102,10 @@ memcached_return_t memcached_server_execute(memcached_st *ptr, #include #include +#ifdef __cplusplus +#include +#endif + /* string value */ struct memcached_continuum_item_st { diff --git a/libmemcached/connect.cc b/libmemcached/connect.cc index 43924116..dee0cd7b 100644 --- a/libmemcached/connect.cc +++ b/libmemcached/connect.cc @@ -135,7 +135,9 @@ static memcached_return_t set_hostinfo(memcached_server_st *server) char str_port[NI_MAXSERV]; int length= snprintf(str_port, NI_MAXSERV, "%u", (uint32_t)server->port); if (length >= NI_MAXSERV || length < 0) + { return MEMCACHED_FAILURE; + } struct addrinfo hints; memset(&hints, 0, sizeof(struct addrinfo)); diff --git a/libmemcached/delete.cc b/libmemcached/delete.cc index 7ef05b07..de4a53dc 100644 --- a/libmemcached/delete.cc +++ b/libmemcached/delete.cc @@ -150,7 +150,8 @@ memcached_return_t memcached_delete_by_key(memcached_st *ptr, if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || send_length < 0) { - rc= MEMCACHED_WRITE_FAILURE; + rc= memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); goto error; } diff --git a/libmemcached/do.cc b/libmemcached/do.cc index 854c6114..5e9e65f2 100644 --- a/libmemcached/do.cc +++ b/libmemcached/do.cc @@ -61,9 +61,10 @@ memcached_return_t memcached_vdo(memcached_server_write_instance_st ptr, WATCHPOINT_ASSERT(count); WATCHPOINT_ASSERT(vector); - if ((rc= memcached_connect(ptr)) != MEMCACHED_SUCCESS) + if (memcached_failed(rc= memcached_connect(ptr))) { WATCHPOINT_ERROR(rc); + assert_msg(ptr->error_messages, "memcached_connect() returned an error but the memcached_server_write_instance_st showed none."); return rc; } @@ -85,7 +86,7 @@ memcached_return_t memcached_vdo(memcached_server_write_instance_st ptr, command_length+= vector->length; } - if (sent_length == -1 || (size_t)sent_length != command_length) + if (sent_length == -1 or size_t(sent_length) != command_length) { rc= MEMCACHED_WRITE_FAILURE; WATCHPOINT_ERROR(rc); diff --git a/libmemcached/dump.cc b/libmemcached/dump.cc index d5aa292a..ef4a0ce3 100644 --- a/libmemcached/dump.cc +++ b/libmemcached/dump.cc @@ -31,7 +31,8 @@ static memcached_return_t ascii_dump(memcached_st *ptr, memcached_dump_fn *callb if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || send_length < 0) { - return MEMCACHED_FAILURE; + return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); } rc= memcached_do(instance, buffer, (size_t)send_length, true); diff --git a/libmemcached/error.cc b/libmemcached/error.cc index a8f42043..1f3b172c 100644 --- a/libmemcached/error.cc +++ b/libmemcached/error.cc @@ -174,6 +174,7 @@ memcached_return_t memcached_set_error(memcached_st& memc, memcached_return_t rc memcached_return_t memcached_set_error(memcached_server_st& self, memcached_return_t rc, const char *at, const char *str, size_t length) { assert(rc != MEMCACHED_ERRNO); + assert(rc != MEMCACHED_SOME_ERRORS); memcached_string_t tmp= { str, length }; return memcached_set_error(self, rc, at, tmp); } @@ -192,6 +193,7 @@ memcached_return_t memcached_set_error(memcached_st& memc, memcached_return_t rc memcached_return_t memcached_set_error(memcached_server_st& self, memcached_return_t rc, const char *at, memcached_string_t& str) { assert(rc != MEMCACHED_ERRNO); + assert(rc != MEMCACHED_SOME_ERRORS); if (memcached_success(rc)) return MEMCACHED_SUCCESS; @@ -223,6 +225,7 @@ memcached_return_t memcached_set_error(memcached_server_st& self, memcached_retu memcached_return_t memcached_set_error(memcached_server_st& self, memcached_return_t rc, const char *at) { assert(rc != MEMCACHED_ERRNO); + assert(rc != MEMCACHED_SOME_ERRORS); if (memcached_success(rc)) return MEMCACHED_SUCCESS; diff --git a/libmemcached/flush.cc b/libmemcached/flush.cc index 6a1364c2..5895f030 100644 --- a/libmemcached/flush.cc +++ b/libmemcached/flush.cc @@ -88,7 +88,8 @@ static memcached_return_t memcached_flush_textual(memcached_st *ptr, if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || send_length < 0) { - return MEMCACHED_FAILURE; + return memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); } rc= memcached_do(instance, buffer, (size_t)send_length, true); diff --git a/libmemcached/hosts.cc b/libmemcached/hosts.cc index db639891..89896e85 100644 --- a/libmemcached/hosts.cc +++ b/libmemcached/hosts.cc @@ -246,7 +246,8 @@ static memcached_return_t update_continuum(memcached_st *ptr) if (sort_host_length >= MEMCACHED_MAX_HOST_SORT_LENGTH || sort_host_length < 0) { - return MEMCACHED_FAILURE; + return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); } #ifdef DEBUG printf("update_continuum: key is %s\n", sort_host); @@ -298,7 +299,8 @@ static memcached_return_t update_continuum(memcached_st *ptr) if (sort_host_length >= MEMCACHED_MAX_HOST_SORT_LENGTH || sort_host_length < 0) { - return MEMCACHED_FAILURE; + return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); } WATCHPOINT_ASSERT(sort_host_length); diff --git a/libmemcached/stats.cc b/libmemcached/stats.cc index 6e2ef31f..05ceb0f2 100644 --- a/libmemcached/stats.cc +++ b/libmemcached/stats.cc @@ -55,7 +55,21 @@ static memcached_return_t set_data(memcached_stat_st *memc_stat, char *key, char } else if (not strcmp("pid", key)) { - memc_stat->pid= strtoul(value, (char **)NULL, 10); + int64_t temp= strtoll(value, (char **)NULL, 10); + + if (temp <= INT32_MAX and ( sizeof(pid_t) == sizeof(int32_t) )) + { + memc_stat->pid= temp; + } + else if (temp > -1) + { + memc_stat->pid= temp; + } + else + { + // If we got a value less then -1 then something went wrong in the + // protocol + } } else if (not strcmp("uptime", key)) { @@ -188,7 +202,7 @@ char *memcached_stat_get_value(const memcached_st *ptr, memcached_stat_st *memc_ if (not memcmp("pid", key, sizeof("pid") -1)) { - length= snprintf(buffer, SMALL_STRING_LEN,"%lu", memc_stat->pid); + length= snprintf(buffer, SMALL_STRING_LEN,"%lld", (signed long long)memc_stat->pid); } else if (not memcmp("uptime", key, sizeof("uptime") -1)) { @@ -384,29 +398,28 @@ static memcached_return_t ascii_stats_fetch(memcached_stat_st *memc_stat, memcached_server_write_instance_st instance, struct local_context *check) { - memcached_return_t rc; char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE]; int send_length; if (args) - send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, - "stats %s\r\n", args); + { + send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, "stats %s\r\n", args); + } else - send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, - "stats\r\n"); + { + send_length= (size_t) snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, "stats\r\n"); + } if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || send_length < 0) - return MEMCACHED_WRITE_FAILURE; - - rc= memcached_do(instance, buffer, (size_t)send_length, true); - if (rc != MEMCACHED_SUCCESS) - goto error; - - while (1) { - rc= memcached_response(instance, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, NULL); + return memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); + } - if (rc == MEMCACHED_STAT) + memcached_return_t rc= memcached_do(instance, buffer, (size_t)send_length, true); + if (memcached_success(rc)) + { + while ((rc= memcached_response(instance, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, NULL)) == MEMCACHED_STAT) { char *string_ptr, *end_ptr; char *key, *value; @@ -439,13 +452,8 @@ static memcached_return_t ascii_stats_fetch(memcached_stat_st *memc_stat, check->context); } } - else - { - break; - } } -error: if (rc == MEMCACHED_END) return MEMCACHED_SUCCESS; else @@ -455,7 +463,7 @@ error: memcached_stat_st *memcached_stat(memcached_st *self, char *args, memcached_return_t *error) { memcached_return_t rc; - if ((rc= initialize_query(self)) != MEMCACHED_SUCCESS) + if (memcached_failed(rc= initialize_query(self))) { if (error) *error= rc; @@ -507,8 +515,10 @@ memcached_stat_st *memcached_stat(memcached_st *self, char *args, memcached_retu temp_return= ascii_stats_fetch(stat_instance, args, instance, NULL); } - if (temp_return != MEMCACHED_SUCCESS) + if (memcached_failed(temp_return)) + { rc= MEMCACHED_SOME_ERRORS; + } } if (error) diff --git a/libmemcached/stats.h b/libmemcached/stats.h index 3f8aa500..f2d064a6 100644 --- a/libmemcached/stats.h +++ b/libmemcached/stats.h @@ -41,7 +41,7 @@ struct memcached_stat_st { unsigned long connection_structures; unsigned long curr_connections; unsigned long curr_items; - unsigned long pid; + pid_t pid; unsigned long pointer_size; unsigned long rusage_system_microseconds; unsigned long rusage_system_seconds; diff --git a/libmemcached/storage.cc b/libmemcached/storage.cc index ffe7faae..9d77f51f 100644 --- a/libmemcached/storage.cc +++ b/libmemcached/storage.cc @@ -118,7 +118,8 @@ static inline memcached_return_t memcached_send(memcached_st *ptr, (ptr->flags.no_reply) ? " noreply" : ""); if (check_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || check_length < 0) { - rc= MEMCACHED_WRITE_FAILURE; + rc= memcached_set_error(*instance, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); memcached_io_reset(instance); return rc; @@ -151,7 +152,8 @@ static inline memcached_return_t memcached_send(memcached_st *ptr, ptr->flags.no_reply ? " noreply" : ""); if ((size_t)check_length >= MEMCACHED_DEFAULT_COMMAND_SIZE -size_t(buffer_ptr - buffer) || check_length < 0) { - rc= MEMCACHED_WRITE_FAILURE; + rc= memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); memcached_io_reset(instance); return rc; diff --git a/libmemcached/util/pid.cc b/libmemcached/util/pid.cc index d9ba79c7..2d7a86dd 100644 --- a/libmemcached/util/pid.cc +++ b/libmemcached/util/pid.cc @@ -56,31 +56,32 @@ pid_t libmemcached_util_getpid(const char *hostname, in_port_t port, memcached_r if (not memc_ptr) { *ret= MEMCACHED_MEMORY_ALLOCATION_FAILURE; - return pid; + return -1; } memcached_return_t rc= memcached_server_add(memc_ptr, hostname, port); if (memcached_success(rc)) { memcached_stat_st *stat= memcached_stat(memc_ptr, NULL, &rc); - if (stat and stat->pid > 0) + if (memcached_success(rc) and stat and stat->pid != -1) { pid= stat->pid; } - else if (memcached_failed(rc) and rc == MEMCACHED_SOME_ERRORS) + else if (memcached_success(rc)) + { + rc= MEMCACHED_UNKNOWN_STAT_KEY; // Something went wrong if this happens + } + else if (rc == MEMCACHED_SOME_ERRORS) // Generic answer, we will now find the specific reason (if one exists) { memcached_server_instance_st instance= memcached_server_instance_by_position(memc_ptr, 0); + assert_msg(instance and instance->error_messages, " "); if (instance and instance->error_messages) { rc= memcached_server_error_return(instance); } } - else if (memcached_success(rc)) - { - rc= MEMCACHED_UNKNOWN_STAT_KEY; // Something went wrong if this happens - } memcached_stat_free(memc_ptr, stat); } diff --git a/libmemcached/util/ping.cc b/libmemcached/util/ping.cc index 839810d4..45f94b19 100644 --- a/libmemcached/util/ping.cc +++ b/libmemcached/util/ping.cc @@ -65,6 +65,7 @@ bool libmemcached_util_ping(const char *hostname, in_port_t port, memcached_retu memcached_server_instance_st instance= memcached_server_instance_by_position(memc_ptr, 0); + assert_msg(instance and instance->error_messages, " "); if (instance and instance->error_messages) { rc= memcached_server_error_return(instance); diff --git a/libmemcached/verbosity.cc b/libmemcached/verbosity.cc index ec00b8de..c4231719 100644 --- a/libmemcached/verbosity.cc +++ b/libmemcached/verbosity.cc @@ -87,7 +87,10 @@ memcached_return_t memcached_verbosity(memcached_st *ptr, uint32_t verbosity) send_length= snprintf(buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, "verbosity %u\r\n", verbosity); if (send_length >= MEMCACHED_DEFAULT_COMMAND_SIZE || send_length < 0) - return MEMCACHED_WRITE_FAILURE; + { + return memcached_set_error(*ptr, MEMCACHED_MEMORY_ALLOCATION_FAILURE, MEMCACHED_AT, + memcached_literal_param("snprintf(MEMCACHED_DEFAULT_COMMAND_SIZE)")); + } struct context_st context = { (size_t)send_length, buffer }; diff --git a/libtest/memcached.cc b/libtest/memcached.cc index 61c3209f..6c019f9d 100644 --- a/libtest/memcached.cc +++ b/libtest/memcached.cc @@ -253,11 +253,3 @@ bool server_startup(server_startup_st *construct) return true; } - -void server_shutdown(server_startup_st *construct) -{ - if (not construct) - return; - - construct->shutdown(); -} diff --git a/libtest/server.cc b/libtest/server.cc index cb15aee9..f82aa299 100644 --- a/libtest/server.cc +++ b/libtest/server.cc @@ -195,3 +195,11 @@ server_startup_st::~server_startup_st() } servers.clear(); } + +void server_shutdown(server_startup_st *construct) +{ + if (not construct) + return; + + construct->shutdown(); +} diff --git a/tests/mem_functions.cc b/tests/mem_functions.cc index 5476f8bd..89ae4f68 100644 --- a/tests/mem_functions.cc +++ b/tests/mem_functions.cc @@ -4396,8 +4396,6 @@ static test_return_t dump_test(memcached_st *memc) return TEST_SUCCESS; } -#ifdef HAVE_LIBMEMCACHEDUTIL - struct test_pool_context_st { memcached_pool_st* pool; memcached_st* mmc; @@ -4539,6 +4537,41 @@ static test_return_t util_version_test(memcached_st *memc) return TEST_SUCCESS; } +static test_return_t getpid_connection_failure_test(memcached_st *memc) +{ + memcached_return_t rc; + memcached_server_instance_st instance= + memcached_server_instance_by_position(memc, 0); + + // Test both the version that returns a code, and the one that does not. + test_true(libmemcached_util_getpid(memcached_server_name(instance), + memcached_server_port(instance) -1, NULL) == -1); + + test_true(libmemcached_util_getpid(memcached_server_name(instance), + memcached_server_port(instance) -1, &rc) == -1); + test_compare_got(MEMCACHED_CONNECTION_FAILURE, rc, memcached_strerror(memc, rc)); + + return TEST_SUCCESS; +} + + +static test_return_t getpid_test(memcached_st *memc) +{ + memcached_return_t rc; + memcached_server_instance_st instance= + memcached_server_instance_by_position(memc, 0); + + // Test both the version that returns a code, and the one that does not. + test_true(libmemcached_util_getpid(memcached_server_name(instance), + memcached_server_port(instance), NULL) > -1); + + test_true(libmemcached_util_getpid(memcached_server_name(instance), + memcached_server_port(instance), &rc) > -1); + test_compare(MEMCACHED_SUCCESS, rc); + + return TEST_SUCCESS; +} + static test_return_t ping_test(memcached_st *memc) { memcached_return_t rc; @@ -4556,7 +4589,6 @@ static test_return_t ping_test(memcached_st *memc) return TEST_SUCCESS; } -#endif #if 0 @@ -6205,7 +6237,6 @@ test_st tests[] ={ {"analyzer", 1, (test_callback_fn*)analyzer_test}, {"connectionpool", 1, (test_callback_fn*)connection_pool_test }, {"memcached_pool_test", 1, (test_callback_fn*)memcached_pool_test }, - {"ping", 1, (test_callback_fn*)ping_test }, {"test_get_last_disconnect", 1, (test_callback_fn*)test_get_last_disconnect}, {"verbosity", 1, (test_callback_fn*)test_verbosity}, {"test_server_failure", 1, (test_callback_fn*)test_server_failure}, @@ -6224,6 +6255,13 @@ test_st behavior_tests[] ={ {0, 0, 0} }; +test_st libmemcachedutil_tests[] ={ + {"libmemcached_util_ping()", 1, (test_callback_fn*)ping_test }, + {"libmemcached_util_getpid()", 1, (test_callback_fn*)getpid_test }, + {"libmemcached_util_getpid(MEMCACHED_CONNECTION_FAILURE)", 1, (test_callback_fn*)getpid_connection_failure_test }, + {0, 0, 0} +}; + test_st basic_tests[] ={ {"init", 1, (test_callback_fn*)basic_init_test}, {"clone", 1, (test_callback_fn*)basic_clone_test}, @@ -6485,6 +6523,7 @@ collection_st collection[] ={ #if 0 {"hash_sanity", 0, 0, hash_sanity}, #endif + {"libmemcachedutil", 0, 0, libmemcachedutil_tests}, {"basic", 0, 0, basic_tests}, {"hsieh_availability", 0, 0, hsieh_availability}, {"murmur_availability", 0, 0, murmur_availability}, -- 2.30.2