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 18a9a316956f6aa3ce676289353eec3221620c9e..85eb3015696868746d667983434818fb8b6e90ec 100644 (file)
--- 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 3e5e77a5795cc29b10d8287fb8260a73a6da1cf1..22bd6f54ef4b53f1cb3a53b74cd8047e4e8b009b 100644 (file)
--- 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 2206c7b852475898ff5505f2d6426bbd68d6a77e..4248e143f7c47e68d0a3c6c82d0a83b2e15458a9 100644 (file)
--- 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 dde59066083c970d3c21105babd0d84c959873de..8fcc11a0fb6b95b2ed95eedd17a9a69735f768f3 100644 (file)
--- 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 27cac75eeed9b1273447b62aa9d41267ba143887..02df660d3b3d686175c412a05f181c3eeec8e8a7 100644 (file)
--- 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 77aa5303bbe2bd580e7c1c1d8670463c8e565d00..cc0fd78aaaf21c8e5614c0776fdf4631ea355d76 100644 (file)
--- 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; \
   } \