This fixes lp:1010899, which showed a condition where we could end up with a bad...

diff --git a/libmemcached-1.0/memcached.hpp b/libmemcached-1.0/memcached.hpp
index 4e1e17780c31faa6e93710137597ccd2a1f0a473..03fbe40239eb1355bad54804d23440e47975fc10 100644 (file)
--- 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 8063be75cb15909498fe36d612b5b6cdf27d4bbf..b8e9e17f52f10e89cab1d8f927e2f5609342212a 100644 (file)
--- 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 02dc26a7922ae59c605a6ca5a37d3090deeafac0..06cd87474cbd0b9da2907a679fa63c85a2209ba7 100644 (file)
--- 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 a01544c5c10fd63a87e7ecdbd8f01bedf6208b81..68f1d7e01800893d010637a04ec9d922ef40bf78 100644 (file)
--- 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<char> 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<test_callback_fn*>(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}
 };