Achieved still yet deeper understanding of CRLF handling in memcached.

  memcached asks for two bytes more than it needs and adds its own CRLF.

  The engine should store the things it needs only.

  *EXCEPT*

  incr has to always add CRLF to the end of the value it stores.

The consequence of this is that we will persist two bytes more than
was strictly required for every record.  The value is opaque to us, so
we're just going to blindly store what memcached told us because *it*
deals with the complexity around the additional CRLF (unless it's an
incr or decr).

Change-Id: I3c6618a596fdd0721589f7a413c5ffc2684a2986
Reviewed-on: http://review.northscale.com:8080/274
Reviewed-by: Sean Lynch <seanl@literati.org>
Tested-by: Eric Lambert <eric.d.lambert@gmail.com>
Tested-by: Dustin Sallings <dustin@spy.net>

ep_engine.h

@@ -708,7 +708,7 @@ friend class LookupCallback;
                 }
 
                 std::stringstream vals;
-                vals << val;
+                vals << val << "\r\n";
                 size_t nb = vals.str().length();
                 *result = val;
                 Item *nit = new Item(key, (uint16_t)nkey, item->getFlags(),
@@ -724,7 +724,7 @@ friend class LookupCallback;
         } else if (ret == ENGINE_KEY_ENOENT && create) {
             std::stringstream vals;
 
-            vals << initial;
+            vals << initial << "\r\n";
             size_t nb = vals.str().length();
 
             *result = initial;

ep_testsuite.c

@@ -142,9 +142,6 @@ static enum test_result check_key_value(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1,
     check(vlen == info.value[0].iov_len, "Length mismatch.");
 
     check(memcmp(info.value[0].iov_base, val, vlen) == 0, "Data mismatch");
-    char *data = (char*)info.value[0].iov_base;
-    check(data[vlen] == '\r', "Missing CR");
-    check(data[vlen+1] == '\n', "Missing LF");
 
     return SUCCESS;
 }
@@ -275,7 +272,21 @@ static enum test_result test_incr_default(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1
                          0) == ENGINE_SUCCESS,
           "Failed third arith.");
     check(result == 3, "Failed third result verification.");
-    return check_key_value(h, h1, "key", "3", 1);
+    return check_key_value(h, h1, "key", "3\r\n", 3);
+}
+
+static enum test_result test_incr(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1) {
+    uint64_t cas = 0, result = 0;
+    item *i = NULL;
+    check(store(h, h1, "cookie", OPERATION_ADD,"key", "1", &i) == ENGINE_SUCCESS,
+          "Failed to add value.");
+
+    check(h1->arithmetic(h, "cookie", "key", 3, true, false, 1, 1, 0,
+                         &cas, &result,
+                         0) == ENGINE_SUCCESS,
+          "Failed to incr value.");
+
+    return check_key_value(h, h1, "key", "2\r\n", 3);
 }
 
 static enum test_result test_flush(ENGINE_HANDLE *h, ENGINE_HANDLE_V1 *h1) {
@@ -427,6 +438,7 @@ engine_test_t* get_tests(void) {
         {"add", test_add, NULL, teardown, NULL},
         {"cas", test_cas, NULL, teardown, NULL},
         {"incr miss", test_incr_miss, NULL, teardown, NULL},
+        {"incr", test_incr, NULL, teardown, NULL},
         {"incr with default", test_incr_default, NULL, teardown, NULL},
         {"delete", test_delete, NULL, teardown, NULL},
         {"flush", test_flush, NULL, teardown, NULL},

item.hh

@@ -71,7 +71,7 @@ public:
     }
 
     uint32_t getNBytes() const {
-        return static_cast<uint32_t>(value->length() - 2);
+        return static_cast<uint32_t>(value->length());
     }
 
     rel_time_t getExptime() const {
@@ -142,7 +142,6 @@ private:
         }
 
         assert(data);
-        data->append("\r\n");
         value.reset(data);
     }
 

t/hash_table_test.cc

@@ -26,7 +26,7 @@ class Counter : public HashTableVisitor {
         std::string key = v->getKey();
         value_t val = v->getValue();
         // Value has length beyond the value that's requested to be stored.
-        std::string subval = val->substr(0, val->length() - 2);
+        std::string subval = val->substr(0, val->length());
         assert(key.compare(subval) == 0);
     }
 };