From: Brian Aker <brian@tangent.org>
Date: Thu, 10 Feb 2011 23:17:29 +0000 (-0800)
Subject: Version will now have a default we can use to look for invalid versions.
X-Git-Tag: 0.51~45^2~5
X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=0b440c1273944e5ad216f1d6fe9e881e66ea0550;p=m6w6%2Flibmemcached

Version will now have a default we can use to look for invalid versions.
Also fixed regression test so that we check for invalid argment return.
---

diff --git a/libmemcached/quit.c b/libmemcached/quit.c
index 18a9a316..85eb3015 100644
--- a/libmemcached/quit.c
+++ b/libmemcached/quit.c
@@ -71,6 +71,10 @@ void memcached_quit_server(memcached_server_st *ptr, bool io_death)
   ptr->options.is_shutting_down= false;
   memcached_server_response_reset(ptr);
 
+  // We reset the version so that if we end up talking to a different server
+  // we don't have stale server version information.
+  ptr->major_version= ptr->minor_version= ptr->micro_version= UINT8_MAX;
+
   if (io_death)
   {
     ptr->server_failure_counter++;
diff --git a/libmemcached/server.c b/libmemcached/server.c
index 3e5e77a5..22bd6f54 100644
--- a/libmemcached/server.c
+++ b/libmemcached/server.c
@@ -32,9 +32,9 @@ static inline void _server_init(memcached_server_st *self, const memcached_st *r
   self->state.is_dead= false;
   WATCHPOINT_SET(self->io_wait_count.read= 0);
   WATCHPOINT_SET(self->io_wait_count.write= 0);
-  self->major_version= 0;
-  self->micro_version= 0;
-  self->minor_version= 0;
+  self->major_version= UINT8_MAX;
+  self->micro_version= UINT8_MAX;
+  self->minor_version= UINT8_MAX;
   self->type= type;
   self->read_ptr= self->read_buffer;
   self->cached_server_error= NULL;
diff --git a/libmemcached/server.h b/libmemcached/server.h
index 2206c7b8..4248e143 100644
--- a/libmemcached/server.h
+++ b/libmemcached/server.h
@@ -36,9 +36,9 @@ struct memcached_server_st {
     uint32_t read;
     uint32_t write;
   } io_wait_count;
-  uint8_t major_version;
-  uint8_t micro_version;
-  uint8_t minor_version;
+  uint8_t major_version; // Default definition of UINT8_MAX means that it has not been set.
+  uint8_t micro_version; // ditto
+  uint8_t minor_version; // ditto
   memcached_connection_t type;
   char *read_ptr;
   char *cached_server_error;
diff --git a/libmemcached/version.c b/libmemcached/version.c
index dde59066..8fcc11a0 100644
--- a/libmemcached/version.c
+++ b/libmemcached/version.c
@@ -45,10 +45,14 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr)
     memcached_server_write_instance_st instance=
       memcached_server_instance_fetch(ptr, x);
 
+    // Optimization, we only fetch version once.
+    if (instance->major_version != UINT8_MAX)
+      continue;
+
     rrc= memcached_do(instance, command, send_length, true);
     if (rrc != MEMCACHED_SUCCESS)
     {
-      instance->major_version= instance->minor_version= instance->micro_version= 0;
+      instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
       rc= MEMCACHED_SOME_ERRORS;
       continue;
     }
@@ -56,7 +60,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr)
     rrc= memcached_response(instance, buffer, MEMCACHED_DEFAULT_COMMAND_SIZE, NULL);
     if (rrc != MEMCACHED_SUCCESS)
     {
-      instance->major_version= instance->minor_version= instance->micro_version= 0;
+      instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
       rc= MEMCACHED_SOME_ERRORS;
       continue;
     }
@@ -68,7 +72,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr)
     instance->major_version= (uint8_t)strtol(response_ptr, (char **)NULL, 10);
     if (errno == ERANGE)
     {
-      instance->major_version= instance->minor_version= instance->micro_version= 0;
+      instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
       rc= MEMCACHED_SOME_ERRORS;
       continue;
     }
@@ -79,7 +83,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr)
     instance->minor_version= (uint8_t)strtol(response_ptr, (char **)NULL, 10);
     if (errno == ERANGE)
     {
-      instance->major_version= instance->minor_version= instance->micro_version= 0;
+      instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
       rc= MEMCACHED_SOME_ERRORS;
       continue;
     }
@@ -89,7 +93,7 @@ static inline memcached_return_t memcached_version_textual(memcached_st *ptr)
     instance->micro_version= (uint8_t)strtol(response_ptr, (char **)NULL, 10);
     if (errno == ERANGE)
     {
-      instance->major_version= instance->minor_version= instance->micro_version= 0;
+      instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
       rc= MEMCACHED_SOME_ERRORS;
       continue;
     }
@@ -114,6 +118,9 @@ static inline memcached_return_t memcached_version_binary(memcached_st *ptr)
     memcached_server_write_instance_st instance=
       memcached_server_instance_fetch(ptr, x);
 
+    if (instance->major_version != UINT8_MAX)
+      continue;
+
     rrc= memcached_do(instance, request.bytes, sizeof(request.bytes), true);
     if (rrc != MEMCACHED_SUCCESS) 
     {
@@ -128,6 +135,9 @@ static inline memcached_return_t memcached_version_binary(memcached_st *ptr)
     memcached_server_write_instance_st instance=
       memcached_server_instance_fetch(ptr, x);
 
+    if (instance->major_version != UINT8_MAX)
+      continue;
+
     if (memcached_server_response_count(instance) > 0) 
     {
       memcached_return_t rrc;
@@ -143,8 +153,29 @@ static inline memcached_return_t memcached_version_binary(memcached_st *ptr)
       }
 
       instance->major_version= (uint8_t)strtol(buffer, &p, 10);
+      if (errno == ERANGE)
+      {
+        instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
+        rc= MEMCACHED_SOME_ERRORS;
+        continue;
+      }
+
       instance->minor_version= (uint8_t)strtol(p + 1, &p, 10);
+      if (errno == ERANGE)
+      {
+        instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
+        rc= MEMCACHED_SOME_ERRORS;
+        continue;
+      }
+
       instance->micro_version= (uint8_t)strtol(p + 1, NULL, 10);
+      if (errno == ERANGE)
+      {
+        instance->major_version= instance->minor_version= instance->micro_version= UINT8_MAX;
+        rc= MEMCACHED_SOME_ERRORS;
+        continue;
+      }
+
     }
   }
 
diff --git a/tests/mem_functions.c b/tests/mem_functions.c
index 27cac75e..02df660d 100644
--- a/tests/mem_functions.c
+++ b/tests/mem_functions.c
@@ -4440,11 +4440,8 @@ static test_return_t util_version_test(memcached_st *memc)
 //  if (! if_successful)
   {
     fprintf(stderr, "\n----------------------------------------------------------------------\n");
-    fprintf(stderr, "\nDumping Server Information\n");
+    fprintf(stderr, "\nDumping Server Information\n\n");
     memcached_server_fn callbacks[1];
-    memcached_version(memc);
-
-    memcached_version(memc);
 
     callbacks[0]= dump_server_information;
     memcached_server_cursor(memc, callbacks, (void *)stderr,  1);
@@ -5613,19 +5610,19 @@ static test_return_t regression_bug_463297(memcached_st *memc)
     memcached_return_t rc= memcached_delete(memc, "foo", 3, 1);
 
     /* but there is a bug in some of the memcached servers (1.4) that treats
-     * the counter as noreply so it doesn't send the proper error message
-   */
-    test_true(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR);
+     * the counter as noreply so it doesn't send the proper error message 
+     */
+    test_true_got(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR || rc == MEMCACHED_INVALID_ARGUMENTS, memcached_strerror(NULL, rc));
 
     /* And buffered mode should be disabled and we should get protocol error */
     test_true(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_BUFFER_REQUESTS, 1) == MEMCACHED_SUCCESS);
     rc= memcached_delete(memc, "foo", 3, 1);
-    test_true(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR);
+    test_true_got(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR || rc == MEMCACHED_INVALID_ARGUMENTS, memcached_strerror(NULL, rc));
 
     /* Same goes for noreply... */
     test_true(memcached_behavior_set(memc, MEMCACHED_BEHAVIOR_NOREPLY, 1) == MEMCACHED_SUCCESS);
     rc= memcached_delete(memc, "foo", 3, 1);
-    test_true(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR);
+    test_true_got(rc == MEMCACHED_PROTOCOL_ERROR || rc == MEMCACHED_NOTFOUND || rc == MEMCACHED_CLIENT_ERROR || rc == MEMCACHED_INVALID_ARGUMENTS, memcached_strerror(NULL, rc));
 
     /* but a normal request should go through (and be buffered) */
     test_true((rc= memcached_delete(memc, "foo", 3, 0)) == MEMCACHED_BUFFERED);
diff --git a/tests/test.h b/tests/test.h
index 77aa5303..cc0fd78a 100644
--- a/tests/test.h
+++ b/tests/test.h
@@ -153,7 +153,7 @@ const char *test_strerror(test_return_t code);
 do \
 { \
   if (1) { \
-    fprintf(stderr, "\nFailed in %s:%d: %s\n", __FILE__, __LINE__, #A);\
+    fprintf(stderr, "\nFailed at %s:%d: %s\n", __FILE__, __LINE__, #A);\
     create_core(); \
     return TEST_FAILURE; \
   } \
@@ -163,7 +163,17 @@ do \
 do \
 { \
   if (! (A)) { \
-    fprintf(stderr, "\nAssertion failed in %s:%d: %s\n", __FILE__, __LINE__, #A);\
+    fprintf(stderr, "\nAssertion failed at %s:%d: %s\n", __FILE__, __LINE__, #A);\
+    create_core(); \
+    return TEST_FAILURE; \
+  } \
+} while (0)
+
+#define test_true_got(A,B) \
+do \
+{ \
+  if (! (A)) { \
+    fprintf(stderr, "\nAssertion failed at %s:%d: \"%s\" received \"%s\"\n", __FILE__, __LINE__, #A, (B));\
     create_core(); \
     return TEST_FAILURE; \
   } \
@@ -173,7 +183,7 @@ do \
 do \
 { \
   if ((A)) { \
-    fprintf(stderr, "\nAssertion failed in %s:%d: %s\n", __FILE__, __LINE__, #A);\
+    fprintf(stderr, "\nAssertion failed at %s:%d: %s\n", __FILE__, __LINE__, #A);\
     create_core(); \
     return TEST_FAILURE; \
   } \