From: Brian Aker Date: Fri, 6 Jul 2012 20:00:13 +0000 (-0700) Subject: This fixes lp:1010899, which showed a condition where we could end up with a bad... X-Git-Tag: 1.0.10~5^2~8 X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=19d7df5119c3f07370f58857f4d916cd5de398ea;p=m6w6%2Flibmemcached This fixes lp:1010899, which showed a condition where we could end up with a bad memc. This does use a trick of reserve and stl::vector to make sure that we have null termination like what we have in the C API. --- diff --git a/libmemcached-1.0/memcached.hpp b/libmemcached-1.0/memcached.hpp index 4e1e1778..03fbe402 100644 --- a/libmemcached-1.0/memcached.hpp +++ b/libmemcached-1.0/memcached.hpp @@ -75,7 +75,7 @@ public: Memcache() { - memc= memcached("", 0); + memc= memcached(NULL, 0); } Memcache(const std::string &config) @@ -85,7 +85,7 @@ public: Memcache(const std::string &hostname, in_port_t port) { - memc= memcached("", 0); + memc= memcached(NULL, 0); if (memc) { memcached_server_add(memc, hostname.c_str(), port); @@ -254,7 +254,8 @@ public: // Actual value, null terminated ret_val.reserve(memcached_result_length(result) +1); ret_val.assign(memcached_result_value(result), - memcached_result_value(result) +memcached_result_length(result)); + memcached_result_value(result) +memcached_result_length(result) +1); + ret_val.resize(memcached_result_length(result)); // Misc flags= memcached_result_flags(result); @@ -292,9 +293,11 @@ public: &value_length, &flags, &rc); if (value != NULL && ret_val.empty()) { - ret_val.reserve(value_length); - ret_val.assign(value, value +value_length); + ret_val.reserve(value_length +1); // Always provide null + ret_val.assign(value, value +value_length +1); + ret_val.resize(value_length); free(value); + return true; } @@ -327,9 +330,11 @@ public: &value_length, &flags, &rc); if (value) { - ret_val.reserve(value_length); - ret_val.assign(value, value +value_length); + ret_val.reserve(value_length +1); // Always provide null + ret_val.assign(value, value +value_length +1); + ret_val.resize(value_length); free(value); + return true; } return false; @@ -399,6 +404,18 @@ public: return memcached_success(rc); } + bool set(const std::string &key, + const char* value, const size_t value_length, + time_t expiration, + uint32_t flags) + { + memcached_return_t rc= memcached_set(memc, + key.c_str(), key.length(), + value, value_length, + expiration, flags); + return memcached_success(rc); + } + /** * Writes an object to a server specified by the master_key parameter. * If the object already exists, it will overwrite the existing object. diff --git a/tests/include.am b/tests/include.am index 8063be75..b8e9e17f 100644 --- a/tests/include.am +++ b/tests/include.am @@ -98,9 +98,6 @@ test-sasl: tests/sasl test-atom: tests/atomsmasher @tests/atomsmasher -test-plus: tests/testplus - @tests/testplus - test-hashplus: tests/hash_plus @tests/hash_plus @@ -122,9 +119,6 @@ gdb-sasl: tests/sasl gdb-atom: tests/atomsmasher @$(DEBUG_COMMAND) tests/atomsmasher -gdb-plus: tests/testplus - $(DEBUG_COMMAND) tests/testplus - gdb-hash: tests/testhashkit @$(DEBUG_COMMAND) tests/testhashkit @@ -146,9 +140,6 @@ valgrind-failure: tests/failure valgrind-atom: tests/atomsmasher $(VALGRIND_COMMAND) tests/atomsmasher -valgrind-plus: tests/testplus - @$(VALGRIND_COMMAND) tests/testplus - valgrind-sasl: tests/sasl @$(VALGRIND_COMMAND) tests/sasl @@ -167,9 +158,6 @@ helgrind-mem: tests/libmemcached-1.0/testapp helgrind-atom: tests/atomsmasher @$(HELGRIND_COMMAND) tests/atomsmasher -helgrind-plus: tests/testplus - @$(HELGRIND_COMMAND) tests/testplus - helgrind-hash: tests/testhashkit @$(HELGRIND_COMMAND) tests/testhashkit diff --git a/tests/libmemcached-1.0/include.am b/tests/libmemcached-1.0/include.am index 02dc26a7..06cd8747 100644 --- a/tests/libmemcached-1.0/include.am +++ b/tests/libmemcached-1.0/include.am @@ -264,3 +264,15 @@ tests_testplus_DEPENDENCIES+= $(TESTS_LDADDS) tests_testplus_LDADD+= $(tests_testplus_DEPENDENCIES) check_PROGRAMS+= tests/testplus noinst_PROGRAMS+= tests/testplus + +test-plus: tests/testplus + @tests/testplus + +gdb-plus: tests/testplus + $(DEBUG_COMMAND) tests/testplus + +valgrind-plus: tests/testplus + @$(VALGRIND_COMMAND) tests/testplus + +helgrind-plus: tests/testplus + @$(HELGRIND_COMMAND) tests/testplus diff --git a/tests/libmemcached-1.0/plus.cpp b/tests/libmemcached-1.0/plus.cpp index a01544c5..68f1d7e0 100644 --- a/tests/libmemcached-1.0/plus.cpp +++ b/tests/libmemcached-1.0/plus.cpp @@ -203,6 +203,36 @@ static test_return_t mget_test(memcached_st *original) return TEST_SUCCESS; } +static test_return_t lp_1010899_TEST(void*) +{ + // Check to see everything is setup internally even when no initial hosts are + // given. + Memcache memc; + + test_false(memc.increment(__func__, 0, NULL)); + + return TEST_SUCCESS; +} + +static test_return_t lp_1010899_with_args_TEST(memcached_st *original) +{ + // Check to see everything is setup internally even when a host is specified + // on creation. + memcached_server_instance_st instance= memcached_server_instance_by_position(original, 0); + Memcache memc(memcached_server_name(instance), memcached_server_port(instance)); + + test_false(memc.increment(__func__, 0, NULL)); + test_true(memc.set(__func__, test_literal_param("12"), 0, 0)); + test_true(memc.increment(__func__, 3, NULL)); + + std::vector ret_val; + test_true(memc.get(__func__, ret_val)); + + test_strcmp(&ret_val[0], "15"); + + return TEST_SUCCESS; +} + static test_return_t basic_behavior(memcached_st *original) { Memcache memc(original); @@ -273,9 +303,17 @@ test_st tests[] ={ {0, 0, 0} }; +test_st regression_TESTS[] ={ + { "lp:1010899 Memcache()", false, lp_1010899_TEST }, + { "lp:1010899 Memcache(localhost, port)", false, + reinterpret_cast(lp_1010899_with_args_TEST) }, + {0, 0, 0} +}; + collection_st collection[] ={ {"block", 0, 0, tests}, {"error()", 0, 0, error_tests}, + {"regression", 0, 0, regression_TESTS}, {0, 0, 0, 0} };