Update from Padraig, one additional valgrind warning I found cleaned up

[m6w6/libmemcached] / libmemcached / memcached_response.c

diff --git a/libmemcached/memcached_response.c b/libmemcached/memcached_response.c
index 554169300fa61a279029d903a7359bb2a16a62c4..fe13ddd53f8045efabdfb51e8fd4fbf583f21555 100644 (file)
--- a/libmemcached/memcached_response.c
+++ b/libmemcached/memcached_response.c
@@ -80,7 +80,6 @@ static memcached_return textual_value_fetch(memcached_server_st *ptr,
   char *end_ptr;
   char *next_ptr;
   size_t value_length;
-  size_t read_length;
   size_t to_read;
   char *value_ptr;
 
@@ -126,7 +125,7 @@ static memcached_return textual_value_fetch(memcached_server_st *ptr,
   if (end_ptr == string_ptr)
     goto read_error;
   for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++);
-  result->flags= strtoul(next_ptr, &string_ptr, 10);
+  result->flags= (uint32_t) strtoul(next_ptr, &string_ptr, 10);
 
   if (end_ptr == string_ptr)
     goto read_error;
@@ -167,7 +166,6 @@ static memcached_return textual_value_fetch(memcached_server_st *ptr,
   }
 
   value_ptr= memcached_string_value(&result->value);
-  read_length= 0;
   /* 
     We read the \r\n into the string since not doing so is more 
     cycles then the waster of memory to do so.
@@ -176,8 +174,12 @@ static memcached_return textual_value_fetch(memcached_server_st *ptr,
     some people lazy about using the return length.
   */
   to_read= (value_length) + 2;
-  read_length= memcached_io_read(ptr, value_ptr, to_read);
-  if (read_length != (size_t)(value_length + 2))
+  ssize_t read_length= 0;
+  memcached_return rrc= memcached_io_read(ptr, value_ptr, to_read, &read_length);
+  if (rrc != MEMCACHED_SUCCESS)
+    return rrc;
+
+  if (read_length != (ssize_t)(value_length + 2))
   {
     goto read_error;
   }
@@ -235,8 +237,36 @@ static memcached_return textual_read_one_response(memcached_server_st *ptr,
         memcached_server_response_increment(ptr);
         return MEMCACHED_STAT;
       }
-      else if (buffer[1] == 'E')
-        return MEMCACHED_SERVER_ERROR;
+      else if (buffer[1] == 'E') /* SERVER_ERROR */ 
+       {
+          char *rel_ptr;
+         char *startptr= buffer + 13, *endptr= startptr;
+
+         while (*endptr != '\r' && *endptr != '\n') endptr++;
+
+          /* 
+            Yes, we could make this "efficent" but to do that we would need
+            to maintain more state for the size of the buffer. Why waste
+            memory in the struct, which is important, for something that
+            rarely should happen?
+          */
+         rel_ptr= (char *)ptr->root->call_realloc(ptr->root, 
+                                                   ptr->cached_server_error, 
+                                                   (size_t) (endptr - startptr + 1));
+
+          if (rel_ptr == NULL)
+          {
+            /* If we happened to have some memory, we just null it since we don't know the size */
+            if (ptr->cached_server_error)
+              ptr->cached_server_error[0]= 0;
+            return MEMCACHED_SERVER_ERROR;
+          }
+         ptr->cached_server_error= rel_ptr;
+
+         memcpy(ptr->cached_server_error, startptr, (size_t) (endptr - startptr));
+         ptr->cached_server_error[endptr - startptr]= 0;
+         return MEMCACHED_SERVER_ERROR;
+       }
       else if (buffer[1] == 'T')
         return MEMCACHED_STORED;
       else
@@ -268,6 +298,10 @@ static memcached_return textual_read_one_response(memcached_server_st *ptr,
       else
         return MEMCACHED_UNKNOWN_READ_FAILURE;
     }
+  case 'I': /* CLIENT ERROR */
+      /* We add back in one because we will need to search for END */
+      memcached_server_response_increment(ptr);
+    return MEMCACHED_ITEM;
   case 'C': /* CLIENT ERROR */
     return MEMCACHED_CLIENT_ERROR;
   default:
@@ -322,8 +356,14 @@ static memcached_return binary_read_one_response(memcached_server_st *ptr,
   {
     switch (header.response.opcode)
     {
-    case PROTOCOL_BINARY_CMD_GETK:
     case PROTOCOL_BINARY_CMD_GETKQ:
+      /*
+       * We didn't increment the response counter for the GETKQ packet
+       * (only the final NOOP), so we need to increment the counter again.
+       */ 
+      memcached_server_response_increment(ptr); 
+      /* FALLTHROUGH */
+    case PROTOCOL_BINARY_CMD_GETK:
       {
         uint16_t keylen= header.response.keylen;
         memcached_result_reset(result);
@@ -389,13 +429,11 @@ static memcached_return binary_read_one_response(memcached_server_st *ptr,
         WATCHPOINT_ASSERT(bodylen == 0);
         return MEMCACHED_SUCCESS;
       } 
-      break;
     case PROTOCOL_BINARY_CMD_NOOP:
       {
         WATCHPOINT_ASSERT(bodylen == 0);
         return MEMCACHED_END;
       }
-      break;
     case PROTOCOL_BINARY_CMD_STAT:
       {
         if (bodylen == 0)
@@ -431,7 +469,7 @@ static memcached_return binary_read_one_response(memcached_server_st *ptr,
       size_t nr= (bodylen > SMALL_STRING_LEN) ? SMALL_STRING_LEN : bodylen;
       if (memcached_safe_read(ptr, hole, nr) != MEMCACHED_SUCCESS)
         return MEMCACHED_UNKNOWN_READ_FAILURE;
-      bodylen -= nr;
+      bodylen-= (uint32_t) nr;
     }
 
     /* This might be an error from one of the quiet commands.. if
@@ -461,11 +499,17 @@ static memcached_return binary_read_one_response(memcached_server_st *ptr,
     case PROTOCOL_BINARY_RESPONSE_KEY_EEXISTS:
       rc= MEMCACHED_DATA_EXISTS;
       break;
+    case PROTOCOL_BINARY_RESPONSE_NOT_STORED:
+      rc= MEMCACHED_NOTSTORED;
+      break;
     case PROTOCOL_BINARY_RESPONSE_E2BIG:
+      rc= MEMCACHED_E2BIG;
+      break;
+    case PROTOCOL_BINARY_RESPONSE_ENOMEM:
+      rc= MEMCACHED_MEMORY_ALLOCATION_FAILURE;
+      break;
     case PROTOCOL_BINARY_RESPONSE_EINVAL:
-    case PROTOCOL_BINARY_RESPONSE_NOT_STORED:
     case PROTOCOL_BINARY_RESPONSE_UNKNOWN_COMMAND:
-    case PROTOCOL_BINARY_RESPONSE_ENOMEM:
     default:
       /* @todo fix the error mappings */
       rc= MEMCACHED_PROTOCOL_ERROR;