INTERNAL: Refactor textual_value_fetch logic

Author: ing-eoking

libmemcached/response.cc

@@ -54,94 +54,70 @@
 #include <libmemcached/common.h>
 #include <libmemcached/string.hpp>
 
+static int tokenize(char *buffer, int length, char **tokens, int ntokens)
+{
+  int i;
+  int cur_str;
+  int begin_str= 0;
+  for (i= 0; i < ntokens; i++)
+  {
+    cur_str= begin_str;
+    while (!iscntrl(buffer[cur_str]) && !isspace(buffer[cur_str]))
+    {
+      if (cur_str >= length) return -1;
+      cur_str++;
+    }
+    if (cur_str == begin_str) break;
+    buffer[cur_str]= '\0';
+    tokens[i]= &buffer[begin_str];
+    begin_str= cur_str + 1;
+  }
+  return i;
+}
+
 static memcached_return_t textual_value_fetch(memcached_server_write_instance_st ptr,
                                               char *buffer, size_t buffer_length,
                                               memcached_result_st *result)
 {
   char *string_ptr;
-  char *end_ptr;
-  char *next_ptr;
-  size_t value_length;
-  size_t to_read;
-  ssize_t read_length= 0;
+  ssize_t value_length;
+  ssize_t read_length;
 
   if (ptr->root->flags.use_udp)
     return memcached_set_error(*ptr, MEMCACHED_NOT_SUPPORTED, MEMCACHED_AT);
 
   WATCHPOINT_ASSERT(ptr->root);
-  end_ptr= buffer + buffer_length;
+
+  string_ptr= buffer + 6; /* "VALUE " */
 
   memcached_result_reset(result);
 
-  string_ptr= buffer;
-  string_ptr+= 6; /* "VALUE " */
+  char *tokens[4]= {NULL, };
+  int ntoken= tokenize(string_ptr, buffer_length - 6, tokens, 4);
+  if (ntoken < 0)
+    return MEMCACHED_PARTIAL_READ;
 
   /* We load the key */
   {
-    char *key= result->item_key;
-    size_t key_length= 0;
-
-    string_ptr += memcached_array_size(ptr->root->_namespace); /* prefix length */
-
-    while (!iscntrl(*string_ptr) && !isspace(*string_ptr)) {
-      if (key_length < MEMCACHED_MAX_KEY) {
-        key[key_length]= *string_ptr;
-      }
-      key_length++;
-      string_ptr++;
-    }
-
-    if (key_length < MEMCACHED_MAX_KEY) {
-      key[key_length]= 0;
-      result->key_length= key_length;
-    } else {
-      snprintf(key + MEMCACHED_MAX_KEY - 4, 4, "...");
-      result->key_length= MEMCACHED_MAX_KEY - 1;
+    char *key= tokens[0] + memcached_array_size(ptr->root->_namespace);
+    int key_length= snprintf(result->item_key, MEMCACHED_MAX_KEY, "%s", key);
+    if (key_length >= MEMCACHED_MAX_KEY)
+    {
+      snprintf(result->item_key + MEMCACHED_MAX_KEY - 4, 4, "...");
+      key_length= MEMCACHED_MAX_KEY - 1;
       memcached_set_error(*ptr, MEMCACHED_KEY_TOO_BIG, MEMCACHED_AT);
     }
-  }
-
-  if (end_ptr == string_ptr)
-    goto read_error;
-
-  /* Flags fetch move past space */
-  string_ptr++;
-  if (end_ptr == string_ptr)
-    goto read_error;
-
-  for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++) {};
-  result->item_flags= (uint32_t) strtoul(next_ptr, &string_ptr, 10);
-
-  if (end_ptr == string_ptr)
-    goto read_error;
 
-  /* Length fetch move past space*/
-  string_ptr++;
-  if (end_ptr == string_ptr)
-    goto read_error;
-
-  for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++) {};
-  value_length= (size_t)strtoull(next_ptr, &string_ptr, 10);
-
-  if (end_ptr == string_ptr)
-    goto read_error;
-
-  /* Skip spaces */
-  if (*string_ptr == '\r')
-  {
-    /* Skip past the \r\n */
-    string_ptr+= 2;
+    result->key_length= (size_t)key_length;
   }
-  else
+
+  result->item_flags= (uint32_t)strtoul(tokens[1], NULL, 10);
+  value_length= (ssize_t)strtoull(tokens[2], NULL, 10);
+  if (ntoken == 4)
   {
-    string_ptr++;
-    for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++) {};
-    result->item_cas= strtoull(next_ptr, &string_ptr, 10);
+    result->item_cas= strtoull(tokens[3], NULL, 10);
   }
 
-  if (end_ptr < string_ptr)
-    goto read_error;
-
   /* We add two bytes so that we can walk the \r\n */
   if (memcached_failed(memcached_string_check(&result->value, value_length +2)))
   {
@@ -150,43 +126,27 @@ static memcached_return_t textual_value_fetch(memcached_server_write_instance_st
 
   {
     char *value_ptr= memcached_string_value_mutable(&result->value);
-    /*
-      We read the \r\n into the string since not doing so is more
-      cycles then the waster of memory to do so.
-
-      We are null terminating through, which will most likely make
-      some people lazy about using the return length.
-    */
-    to_read= (value_length) + 2;
-    memcached_return_t rrc= memcached_io_read(ptr, value_ptr, to_read, &read_length);
-    if (memcached_failed(rrc))
+    memcached_return_t rc= memcached_io_read(ptr, value_ptr, value_length + 2, &read_length);
+    if (rc == MEMCACHED_IN_PROGRESS)
     {
-      if (rrc == MEMCACHED_IN_PROGRESS) {
-        memcached_quit_server(ptr, true);
-        return memcached_set_error(*ptr, rrc, MEMCACHED_AT);
-      }
-      return rrc;
+      memcached_quit_server(ptr, true);
+      return memcached_set_error(*ptr, rc, MEMCACHED_AT);
+    }
+    else if (memcached_failed(rc))
+    {
+      return rc;
     }
-  }
 
-  if (read_length != (ssize_t)(value_length + 2))
-  {
-    goto read_error;
-  }
+    if (read_length != value_length + 2)
+    {
+      return MEMCACHED_PARTIAL_READ;
+    }
 
-  /* This next bit blows the API, but this is internal....*/
-  {
-    char *char_ptr;
-    char_ptr= memcached_string_value_mutable(&result->value);;
-    char_ptr[value_length]= 0;
-    char_ptr[value_length +1]= 0;
+    memset(value_ptr + value_length, 0, 2);
     memcached_string_set_length(&result->value, value_length);
   }
 
   return MEMCACHED_SUCCESS;
-
-read_error:
-  return MEMCACHED_PARTIAL_READ;
 }
 
 static memcached_return_t textual_version_fetch(memcached_server_write_instance_st instance, char *buffer)