Cleanup result and add more tests.

ChangeLog

@@ -1,6 +1,8 @@
 1.0.4
 * Added --coredump flag to gearmand
 
+* Removed result type enum, since the structure is never exposed to the user this was of limited use.
+
 1.0.3 Sun Feb  3 21:05:52 EST 2013
 * Allow for a longer set of retries if port is in use when the server starts up.
 * Added checking for valgrind sgcheck

libgearman-1.0/result.h

@@ -37,15 +37,6 @@
 
 #pragma once
 
-enum gearman_result_t {
-  GEARMAN_RESULT_BINARY,
-  GEARMAN_RESULT_BOOLEAN,
-  GEARMAN_RESULT_INTEGER
-};
-#ifndef __cplusplus
-typedef enum gearman_result_t gearman_result_t;
-#endif
-
 #ifdef __cplusplus
 extern "C" {
 #endif
@@ -59,9 +50,6 @@ extern "C" {
   GEARMAN_API
   gearman_string_t gearman_result_string(const gearman_result_st *self);
 
-  GEARMAN_LOCAL
-  gearman_string_t gearman_result_take_string(gearman_result_st *self);
-
   GEARMAN_API
   gearman_return_t gearman_result_store_string(gearman_result_st *self, gearman_string_t arg);
 

libgearman/include.am

@@ -81,6 +81,13 @@ libgearman_libgearman_vector_la_LIBADD= @DL_LIB@
 libgearman_libgearman_vector_la_SOURCES= libgearman/vector.cc
 libgearman_libgearman_vector_la_SOURCES+= libgearman/backtrace.cc
 
+noinst_LTLIBRARIES+= libgearman/libgearman-result.la
+libgearman_libgearman_result_la_CXXFLAGS= -DBUILDING_LIBGEARMAN
+libgearman_libgearman_result_la_LIBADD= @DL_LIB@
+libgearman_libgearman_result_la_SOURCES= libgearman/vector.cc
+libgearman_libgearman_result_la_SOURCES+= libgearman/result.cc
+libgearman_libgearman_result_la_SOURCES+= libgearman/backtrace.cc
+
 libgearman_libgearman_la_LIBADD=
 libgearman_libgearman_la_SOURCES=
 libgearman_libgearman_la_CXXFLAGS=

libgearman/result.cc

@@ -36,10 +36,13 @@
  */
 
 #include "gear_config.h"
-#include <libgearman/common.h>
 
+#include "libgearman/result.hpp"
 #include "libgearman/assert.hpp"
 
+#include "libgearman-1.0/visibility.h"
+#include "libgearman-1.0/result.h"
+
 #include <cstdlib>
 #include <limits>
 #include <memory>
@@ -47,39 +50,94 @@
 #include <libgearman/result.hpp>
 
 gearman_result_st::gearman_result_st() :
-  _is_null(true),
-  type(GEARMAN_RESULT_BOOLEAN)
+  type(GEARMAN_RESULT_NULL)
 {
-  value.boolean= false;
+  value._boolean= false;
 }
 
-gearman_result_st::gearman_result_st(size_t initial_size) :
-  _is_null(true),
-  type(GEARMAN_RESULT_BINARY),
-  value(initial_size)
+gearman_result_st::gearman_result_st(size_t reserve_size_) :
+  type(GEARMAN_RESULT_NULL),
+  value(reserve_size_)
 {
 }
 
 bool gearman_result_is_null(const gearman_result_st *self)
 {
-  return self->is_null();
+  assert(self);
+  if (self)
+  {
+    return self->is_null();
+  }
+
+  return true;
+}
+
+size_t gearman_result_st::size() const
+{
+  switch (type)
+  {
+  case GEARMAN_RESULT_BINARY:
+    return value.string.size();
+
+  case GEARMAN_RESULT_BOOLEAN:
+    return 1;
+
+  case GEARMAN_RESULT_INTEGER:
+    return sizeof(int64_t);
+
+  case GEARMAN_RESULT_NULL:
+    return 0;
+  }
+
+  return 0;
+}
+
+int64_t gearman_result_st::integer() const
+{
+  switch (type)
+  {
+  case GEARMAN_RESULT_BINARY:
+    return atoll(value.string.value());
+
+  case GEARMAN_RESULT_BOOLEAN:
+    return value._boolean;
+
+  case GEARMAN_RESULT_INTEGER:
+    return value._integer;
+
+  case GEARMAN_RESULT_NULL:
+    return 0;
+  }
+
+  return 0;
 }
 
 int64_t gearman_result_integer(const gearman_result_st *self)
 {
+  assert(self);
   if (self)
   {
-    switch (self->type)
-    {
-    case GEARMAN_RESULT_BINARY:
-      return atoll(gearman_string_value(&self->value.string));
+    return self->integer();
+  }
 
-    case GEARMAN_RESULT_BOOLEAN:
-      return self->value.boolean;
+  return 0;
+}
 
-    case GEARMAN_RESULT_INTEGER:
-      return self->value.integer;
-    }
+bool gearman_result_st::boolean() const
+{
+  switch (type)
+  {
+  case GEARMAN_RESULT_BINARY:
+    return value.string.size();
+
+  case GEARMAN_RESULT_BOOLEAN:
+    return value._boolean;
+
+  case GEARMAN_RESULT_INTEGER:
+    return value._integer ? true : false;
+
+  case GEARMAN_RESULT_NULL:
+    return false;
   }
 
   return false;
@@ -89,17 +147,7 @@ bool gearman_result_boolean(const gearman_result_st *self)
 {
   if (self)
   {
-    switch (self->type)
-    {
-    case GEARMAN_RESULT_BINARY:
-      return gearman_string_length(&self->value.string);
-
-    case GEARMAN_RESULT_BOOLEAN:
-      return self->value.boolean;
-
-    case GEARMAN_RESULT_INTEGER:
-      return self->value.integer ? true : false;
-    }
+    return self->boolean();
   }
 
   return false;
@@ -143,8 +191,7 @@ gearman_string_t gearman_result_take_string(gearman_result_st *self)
   if (self->type == GEARMAN_RESULT_BINARY and gearman_result_size(self))
   {
     gearman_string_t ret_string= gearman_string_take_string(&self->value.string);
-    self->type= GEARMAN_RESULT_BOOLEAN; // Set to default type
-    self->_is_null= true;
+    self->type= GEARMAN_RESULT_NULL; // Set to NULL
 
     return ret_string;
   }
@@ -155,42 +202,96 @@ gearman_string_t gearman_result_take_string(gearman_result_st *self)
 
 gearman_return_t gearman_result_store_string(gearman_result_st *self, gearman_string_t arg)
 {
-  return gearman_result_store_value(self, gearman_string_param(arg));
+  assert(self);
+  if (self)
+  {
+    if (self->store(gearman_string_param(arg)) == false)
+    {
+      return GEARMAN_MEMORY_ALLOCATION_FAILURE;
+    }
+
+    return GEARMAN_SUCCESS;
+  }
+
+  return GEARMAN_INVALID_ARGUMENT;
 }
 
-gearman_return_t gearman_result_store_value(gearman_result_st *self, const void *value, size_t size)
+bool gearman_result_st::append(const char* arg, const size_t arg_length)
 {
-  if (self == NULL)
+  if (type != GEARMAN_RESULT_BINARY)
+  {
+    clear();
+    type= GEARMAN_RESULT_BINARY;
+  }
+
+  if (value.string.append(arg, arg_length) == false)
   {
-    return GEARMAN_INVALID_ARGUMENT;
+    type= GEARMAN_RESULT_NULL;
+    return false;
   }
 
-  self->value.string.clear();
-  if (value)
+  return true;
+}
+
+bool gearman_result_st::store(const char* arg, const size_t arg_length)
+{
+  value.string.clear();
+  if (gearman_string_append(&value.string, arg, arg_length) == false)
   {
-    if (gearman_string_append(&self->value.string, static_cast<const char *>(value), size) == false)
+    type= GEARMAN_RESULT_NULL;
+    return false;
+  }
+
+  type= GEARMAN_RESULT_BINARY;
+
+  return true;
+}
+
+gearman_return_t gearman_result_store_value(gearman_result_st *self, const void *value, size_t size)
+{
+  if (self)
+  {
+    if (self->type != GEARMAN_RESULT_BINARY)
+    {
+      self->clear();
+    }
+
+    if (self->store((const char*)value, size) == false) // If append should fail, we default to NULL
     {
+      self->type= GEARMAN_RESULT_NULL;
       return GEARMAN_MEMORY_ALLOCATION_FAILURE;
     }
+    self->type= GEARMAN_RESULT_BINARY;
+
+    return GEARMAN_SUCCESS;
   }
 
-  self->_is_null= false;
-  self->type= GEARMAN_RESULT_BINARY;
+  return GEARMAN_INVALID_ARGUMENT;
+}
 
-  return GEARMAN_SUCCESS;
+void gearman_result_st::integer(int64_t arg_)
+{
+  if (type != GEARMAN_RESULT_INTEGER)
+  {
+    clear();
+    type= GEARMAN_RESULT_INTEGER;
+  }
+
+  value._integer= arg_;
 }
 
-void gearman_result_store_integer(gearman_result_st *self, int64_t value)
+void gearman_result_store_integer(gearman_result_st *self, int64_t arg_)
 {
   if (self)
   {
-    if (self->type == GEARMAN_RESULT_BINARY)
-    {
-      self->value.string.clear();
-    }
+    self->integer(arg_);
+  }
+}
 
-    self->type= GEARMAN_RESULT_INTEGER;
-    self->value.integer= value;
-    self->_is_null= false;
+void gearman_result_store_boolean(gearman_result_st *self, const bool arg_)
+{
+  if (self)
+  {
+    self->boolean(arg_);
   }
 }

libgearman/result.hpp

@@ -39,26 +39,37 @@
 #pragma once
 
 #include "libgearman/assert.hpp"
+#include "libgearman/vector.hpp"
+#include "libgearman-1.0/return.h"
+
+#include <cstddef>
+#include <cstdlib>
 #include <cstring>
 
+enum gearman_result_t {
+  GEARMAN_RESULT_BINARY,
+  GEARMAN_RESULT_BOOLEAN,
+  GEARMAN_RESULT_INTEGER,
+  GEARMAN_RESULT_NULL
+};
+
 struct gearman_result_st
 {
-  bool _is_null;
   enum gearman_result_t type;
 
   struct Value {
-    bool boolean;
-    int64_t integer;
+    bool _boolean;
+    int64_t _integer;
     gearman_vector_st string;
 
     Value() :
-      boolean(false),
-      integer(0)
+      _boolean(false),
+      _integer(0)
     { }
 
     Value(size_t initial_size) :
-      boolean(false),
-      integer(0),
+      _boolean(false),
+      _integer(0),
       string(initial_size)
     { }
 
@@ -66,33 +77,45 @@ struct gearman_result_st
 
   gearman_result_st();
 
-  explicit gearman_result_st(size_t initial_size);
+  explicit gearman_result_st(size_t);
 
   bool is_null() const
   {
-    return _is_null;
+    return type == GEARMAN_RESULT_NULL ? true : false;
   }
 
   void clear()
   {
-    if (type == GEARMAN_RESULT_BINARY)
-    {
-      value.string.clear();
-    }
-    else if (type == GEARMAN_RESULT_INTEGER)
-    {
-      value.integer= 0;
-    }
+    value.string.clear();
+    value._integer= 0;
+    value._boolean= false;
+    type= GEARMAN_RESULT_NULL;
+  }
+
+  bool store(const char*, const size_t);
+  bool append(const char*, const size_t);
+
+  size_t size() const;
 
-    type= GEARMAN_RESULT_BOOLEAN;
-    value.boolean= false;
-    _is_null= true;
+  size_t capacity() const
+  {
+    return value.string.capacity();
+  }
+
+  void resize(size_t arg_)
+  {
+    return value.string.resize(arg_);
+  }
+
+  void reserve(size_t arg_)
+  {
+    return value.string.reserve(arg_);
   }
 
   gearman_vector_st *mutable_string()
   {
-    value.integer= 0;
-    value.boolean= false;
+    value._integer= 0;
+    value._boolean= false;
     type= GEARMAN_RESULT_BINARY;
 
     return &value.string;
@@ -108,16 +131,22 @@ struct gearman_result_st
     return NULL;
   }
 
-  int64_t integer()
+  void boolean(bool arg_)
   {
-    if (type == GEARMAN_RESULT_INTEGER)
+    if (type != GEARMAN_RESULT_BOOLEAN)
     {
-      return value.integer;
+      clear();
+      type= GEARMAN_RESULT_BOOLEAN;
     }
 
-    return 0;
+    value._boolean= arg_;
   }
 
+  bool boolean() const;
+
+  int64_t integer() const;
+  void integer(int64_t);
+
   ~gearman_result_st()
   {
     gearman_string_free(&value.string);
@@ -127,3 +156,6 @@ private:
   gearman_result_st( const gearman_result_st& );
   const gearman_result_st& operator=( const gearman_result_st& );
 };
+
+
+gearman_string_t gearman_result_take_string(gearman_result_st *self);

libgearman/vector.cc

@@ -236,6 +236,17 @@ void gearman_string_clear(gearman_vector_st *string)
   string->clear();
 }
 
+bool gearman_vector_st::store(const char* arg_, const size_t arg_length_)
+{
+  clear();
+  return append(arg_, arg_length_);
+}
+
+bool gearman_vector_st::append(const char* arg_, const size_t arg_length_)
+{
+  return gearman_string_append(this, arg_, arg_length_);
+}
+
 gearman_vector_st::~gearman_vector_st()
 {
   if (string)

libgearman/vector.hpp

@@ -78,7 +78,6 @@ struct gearman_vector_st {
   void resize(size_t);
   void reserve(size_t);
 
-
   void clear()
   {
     end= string;
@@ -88,6 +87,11 @@ struct gearman_vector_st {
     }
   }
 
+  const char* value() const
+  {
+    return string;
+  }
+
   size_t capacity() const
   {
     // We tell a white lie about size since we always keep things null
@@ -100,6 +104,9 @@ struct gearman_vector_st {
     return current_size;
   }
 
+  bool store(const char*, const size_t);
+  bool append(const char* arg_, const size_t arg_length_);
+
   size_t size() const;
 
   void init();

tests/include.am

@@ -63,6 +63,23 @@ test-vector: tests/vector
 valgrind-vector: tests/vector
 	@$(VALGRIND_COMMAND) tests/vector
 
+# Vector tests
+tests_result_SOURCES=
+tests_result_LDADD=
+
+tests_result_SOURCES+= tests/result.cc
+tests_result_LDADD+= libtest/libtest.la
+tests_result_LDADD+= libgearman/libgearman-result.la
+check_PROGRAMS+= tests/result
+noinst_PROGRAMS+= tests/result
+
+test-result: tests/result
+	@tests/result
+
+valgrind-result: tests/result
+	@$(VALGRIND_COMMAND) tests/result
+
+
 EXTRA_tests_cli_DEPENDENCIES=
 tests_cli_SOURCES=
 tests_cli_LDADD=

tests/result.cc

@@ -0,0 +1,256 @@
+/*  vim:expandtab:shiftwidth=2:tabstop=2:smarttab:
+ * 
+ *  Test memcached_result_st
+ *
+ *  Copyright (C) 2013 Data Differential, http://datadifferential.com/
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions are
+ *  met:
+ *
+ *      * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ *
+ *      * Redistributions in binary form must reproduce the above
+ *  copyright notice, this list of conditions and the following disclaimer
+ *  in the documentation and/or other materials provided with the
+ *  distribution.
+ *
+ *      * The names of its contributors may not be used to endorse or
+ *  promote products derived from this software without specific prior
+ *  written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *  OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+
+/*
+  Test that we are cycling the servers we are creating during testing.
+*/
+
+#include "gear_config.h"
+
+#include <libtest/test.hpp>
+using namespace libtest;
+
+#include "libgearman/result.hpp"
+#include "libgearman/assert.hpp"
+
+#include "libgearman-1.0/visibility.h"
+#include "libgearman-1.0/result.h"
+
+static test_return_t declare_result_TEST(void*)
+{
+  gearman_result_st result;
+  ASSERT_EQ(0, result.size());
+  ASSERT_EQ(0, result.capacity());
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t new_result_TEST(void*)
+{
+  gearman_result_st* result= new gearman_result_st;
+  ASSERT_EQ(0, result->size());
+  ASSERT_EQ(0, result->capacity());
+
+  delete result;
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t declare_result_size_TEST(void*)
+{
+  gearman_result_st result(2048);
+  ASSERT_EQ(0, result.size());
+  ASSERT_TRUE(result.capacity() >= 2048);
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t new_result_size_TEST(void*)
+{
+  gearman_result_st* result= new gearman_result_st(2023);
+
+  ASSERT_EQ(0, result->size());
+  ASSERT_TRUE(result->capacity() >= 2023);
+
+  delete result;
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t zero_resize_TEST(void*)
+{
+  gearman_result_st result(89);
+  ASSERT_EQ(0, result.size());
+  ASSERT_TRUE(result.capacity() >= 89);
+
+  result.resize(0);
+  ASSERT_EQ(0, result.size());
+  ASSERT_EQ(0, result.capacity());
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t smaller_resize_TEST(void*)
+{
+  gearman_result_st result(89);
+  ASSERT_EQ(0, result.size());
+  ASSERT_TRUE(result.capacity() >= 89);
+
+  result.resize(20);
+  ASSERT_EQ(0, result.size());
+  ASSERT_TRUE(result.capacity() >= 20);
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t bigger_resize_TEST(void*)
+{
+  gearman_result_st result(89);
+  ASSERT_EQ(0, result.size());
+  ASSERT_TRUE(result.capacity() >= 89);
+
+  result.resize(181);
+  ASSERT_EQ(0, result.size());
+  ASSERT_TRUE(result.capacity() >= 181);
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t random_resize_TEST(void*)
+{
+  const size_t max_block= 10 * GEARMAN_VECTOR_BLOCK_SIZE;
+  for (size_t x= 0; x < 20; x++)
+  {
+    gearman_result_st result(random() % max_block);
+    ASSERT_TRUE(result.capacity() >= result.size());
+    result.resize(random() % max_block);
+    ASSERT_TRUE(result.capacity() >= result.size());
+    result.resize(random() % max_block +GEARMAN_VECTOR_BLOCK_SIZE);
+    ASSERT_TRUE(result.capacity() >= result.size());
+  }
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t append_TEST(void*)
+{
+  const size_t max_block= 10 * GEARMAN_VECTOR_BLOCK_SIZE;
+  for (size_t x= 0; x < 20; x++)
+  {
+    gearman_result_st result(random() % max_block);
+    libtest::vchar_t random_string;
+    libtest::vchar::make(random_string, random() % max_block);
+    result.append(&random_string[0], random_string.size());
+
+    if (random() % 2)
+    {
+      result.clear();
+    }
+  }
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t gearman_string_take_TEST(void*)
+{
+  const size_t max_block= 10 * GEARMAN_VECTOR_BLOCK_SIZE;
+  for (size_t x= 0; x < 20; x++)
+  {
+    gearman_result_st result((random() % max_block) +1);
+
+    // Now we insert a random string
+    libtest::vchar_t random_string;
+    libtest::vchar::make(random_string, random() % max_block);
+    result.append(&random_string[0], random_string.size());
+
+    gearman_string_t temp= gearman_result_take_string(&result);
+    ASSERT_TRUE(gearman_c_str(temp));
+    free((void*)(gearman_c_str(temp)));
+  }
+
+  return TEST_SUCCESS;
+}
+
+static test_return_t gearman_string_allocate_take_TEST(void*)
+{
+  const size_t max_block= 10 * GEARMAN_VECTOR_BLOCK_SIZE;
+  for (size_t x= 0; x < 20; x++)
+  {
+    gearman_result_st* result= new gearman_result_st((random() % max_block) +1);
+
+    { // Now we insert a random string
+      libtest::vchar_t random_string;
+      libtest::vchar::make(random_string, random() % max_block);
+      result->append(&random_string[0], random_string.size());
+    }
+
+    gearman_string_t temp= gearman_result_take_string(result);
+    ASSERT_TRUE(gearman_c_str(temp));
+    free((void*)(gearman_c_str(temp)));
+
+    if (random() % 2)
+    { // Now we insert a random string
+      libtest::vchar_t random_string;
+      libtest::vchar::make(random_string, random() % max_block);
+      result->append(&random_string[0], random_string.size());
+    }
+
+    delete result;
+  }
+
+  return TEST_SUCCESS;
+}
+
+test_st allocate_TESTS[] ={
+  { "declare result", 0, declare_result_TEST },
+  { "new result", 0, new_result_TEST },
+  { "declare result(2048)", 0, declare_result_size_TEST },
+  { "new result(2023)", 0, new_result_size_TEST },
+  { 0, 0, 0 }
+};
+
+test_st resize_TESTS[] ={
+  { "zero", 0, zero_resize_TEST },
+  { "smaller", 0, smaller_resize_TEST },
+  { "bigger", 0, bigger_resize_TEST },
+  { "random", 0, random_resize_TEST },
+  { 0, 0, 0 }
+};
+
+test_st append_TESTS[] ={
+  { "append()", 0, append_TEST },
+  { 0, 0, 0 }
+};
+
+test_st take_TESTS[] ={
+  { "gearman_string_take_string()", 0, gearman_string_take_TEST },
+  { "new gearman_result_st() gearman_string_take_string()", 0, gearman_string_allocate_take_TEST },
+  { 0, 0, 0 }
+};
+
+collection_st collection[] ={
+  {"allocate", NULL, NULL, allocate_TESTS },
+  {"resize", NULL, NULL, resize_TESTS },
+  {"append", NULL, NULL, append_TESTS },
+  {"take", NULL, NULL, take_TESTS },
+  {0, 0, 0, 0}
+};
+
+void get_world(libtest::Framework *world)
+{
+  world->collections(collection);
+}