Merge lp:~brianaker/gearmand/gearman_connection_create-cleanup into lp:gearmand

[Brian Aker]

Cleanup patch

=== modified file 'libgearman/add.cc'
--- libgearman/add.cc	2013-06-11 06:48:11 +0000
+++ libgearman/add.cc	2013-06-25 23:50:39 +0000
@@ -286,8 +286,8 @@
 
   case GEARMAN_COMMAND_SUBMIT_REDUCE_JOB:
   case GEARMAN_COMMAND_SUBMIT_REDUCE_JOB_BACKGROUND:
-    assert(0);
     rc= GEARMAN_INVALID_ARGUMENT;
+    assert(rc != GEARMAN_INVALID_ARGUMENT);
     break;
 
   case GEARMAN_COMMAND_SUBMIT_JOB_SCHED:
@@ -325,8 +325,8 @@
   case GEARMAN_COMMAND_WORK_WARNING:
   case GEARMAN_COMMAND_GET_STATUS_UNIQUE:
   case GEARMAN_COMMAND_STATUS_RES_UNIQUE:
-    assert(0);
     rc= GEARMAN_INVALID_ARGUMENT;
+    assert(rc != GEARMAN_INVALID_ARGUMENT);
     break;
   }
 

=== modified file 'libgearman/backtrace.cc'
--- libgearman/backtrace.cc	2013-01-17 19:38:43 +0000
+++ libgearman/backtrace.cc	2013-06-25 23:50:39 +0000
@@ -51,13 +51,12 @@
 
 #ifdef HAVE_GCC_ABI_DEMANGLE
 # include <cxxabi.h>
-# define USE_DEMANGLE 1
-#else
-# define USE_DEMANGLE 0
-#endif
-
-#ifdef HAVE_DLFCN_H
-# include <dlfcn.h>
+# ifdef HAVE_DLFCN_H
+#  define USE_DEMANGLE 1
+#  include <dlfcn.h>
+# else
+#  define USE_DEMANGLE 0
+# endif   
 #endif   
 
 const int MAX_DEPTH= 50;
@@ -78,9 +77,9 @@
       {
         bool was_demangled= false;
 
+#if defined(USE_DEMANGLE)
         if (USE_DEMANGLE)
         {
-#ifdef HAVE_DLFCN_H
           Dl_info dlinfo;
           if (dladdr(backtrace_buffer[x], &dlinfo))
           {
@@ -108,8 +107,8 @@
                       dlinfo.dli_fname);
             }
           }
-#endif
         }
+#endif // USE_DEMANGLE
 
         if (was_demangled == false)
         {

=== modified file 'libgearman/client.cc'
--- libgearman/client.cc	2013-06-15 09:06:53 +0000
+++ libgearman/client.cc	2013-06-25 23:50:39 +0000
@@ -593,7 +593,7 @@
 {
   if (client_shell and client_shell->impl())
   {
-    if (gearman_connection_create_args(client_shell->impl()->universal, host, port) == false)
+    if (gearman_connection_create(client_shell->impl()->universal, host, port) == false)
     {
       assert(client_shell->impl()->universal.error_code() != GEARMAN_SUCCESS);
       return client_shell->impl()->universal.error_code();
@@ -608,7 +608,7 @@
 
 gearman_return_t Client::add_server(const char *host, const char* service_)
 {
-  if (gearman_connection_create_args(universal, host, service_) == false)
+  if (gearman_connection_create(universal, host, service_) == false)
   {
     assert(universal.error_code() != GEARMAN_SUCCESS);
     return universal.error_code();

=== modified file 'libgearman/command.cc'
--- libgearman/command.cc	2013-05-11 23:09:32 +0000
+++ libgearman/command.cc	2013-06-25 23:50:39 +0000
@@ -109,7 +109,7 @@
   }
 
   assert(0); // We should never reach this
-  abort();
+  return "__UNKNOWN";
 }
 
 const gearman_command_info_st *gearman_command_info(gearman_command_t command)

=== modified file 'libgearman/connection.cc'
--- libgearman/connection.cc	2013-06-14 21:52:52 +0000
+++ libgearman/connection.cc	2013-06-25 23:50:39 +0000
@@ -79,35 +79,6 @@
 # define MSG_DONTWAIT 0
 #endif
 
-static gearman_return_t gearman_connection_set_option(gearman_connection_st *connection,
-                                                      gearman_connection_options_t options,
-                                                      bool value)
-{
-  switch (options)
-  {
-  case GEARMAN_CON_READY:
-    connection->options.ready= value;
-    break;
-
-  case GEARMAN_CON_PACKET_IN_USE:
-    connection->options.packet_in_use= value;
-    break;
-
-  case GEARMAN_CON_IGNORE_LOST_CONNECTION:
-    break;
-
-  case GEARMAN_CON_CLOSE_AFTER_FLUSH:
-    break;
-
-  case GEARMAN_CON_EXTERNAL_FD:
-  case GEARMAN_CON_MAX:
-  default:
-    return GEARMAN_INVALID_COMMAND;
-  }
-
-  return GEARMAN_SUCCESS;
-}
-
 
 /**
  * @addtogroup gearman_connection_static Static Connection Declarations
@@ -115,8 +86,7 @@
  * @{
  */
 
-gearman_connection_st::gearman_connection_st(gearman_universal_st &universal_arg,
-                                             gearman_connection_options_t *options_args) :
+gearman_connection_st::gearman_connection_st(gearman_universal_st &universal_arg) :
   state(GEARMAN_CON_UNIVERSAL_ADDRINFO),
   send_state(GEARMAN_CON_SEND_STATE_NONE),
   recv_state(GEARMAN_CON_RECV_UNIVERSAL_NONE),
@@ -142,15 +112,6 @@
   send_buffer_ptr(NULL),
   _recv_packet(NULL)
 {
-  if (options_args)
-  {
-    while (*options_args != GEARMAN_CON_MAX)
-    {
-      gearman_connection_set_option(this, *options_args, true);
-      options_args++;
-    }
-  }
-
   if (universal.con_list)
   {
     universal.con_list->prev= this;
@@ -165,10 +126,14 @@
   _service[0]= 0;
 }
 
-gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
-                                                 gearman_connection_options_t *options)
+/**
+ * Initialize a connection structure. Always check the return value even if
+ * passing in a pre-allocated structure. Some other initialization may have
+ * failed.
+ */
+static gearman_connection_st *__connection_create(gearman_universal_st &universal)
 {
-  gearman_connection_st *connection= new (std::nothrow) gearman_connection_st(universal, options);
+  gearman_connection_st *connection= new (std::nothrow) gearman_connection_st(universal);
   if (connection == NULL)
   {
     gearman_perror(universal, "Failed at new() gearman_connection_st new");
@@ -177,38 +142,34 @@
   return connection;
 }
 
-gearman_connection_st *gearman_connection_create_args(gearman_universal_st& universal,
-                                                      const char *host, const char* service_)
+gearman_connection_st *gearman_connection_create(gearman_universal_st& universal,
+                                                 const char *host, const char* service_)
 {
-  gearman_connection_st *connection= gearman_connection_create(universal);
-  if (connection == NULL)
-  {
-    return NULL;
-  }
-
-  connection->set_host(host, service_);
-
-  if (gearman_failed(connection->lookup()))
-  {
-    gearman_gerror(universal, GEARMAN_GETADDRINFO);
-    delete connection;
-    return NULL;
+  gearman_connection_st *connection= __connection_create(universal);
+  if (connection)
+  {
+    connection->set_host(host, (const char*)service_);
+
+    if (gearman_failed(connection->lookup()))
+    {
+      delete connection;
+      return NULL;
+    }
   }
 
   return connection;
 }
 
-gearman_connection_st *gearman_connection_create_args(gearman_universal_st& universal,
-                                                      const char *host, in_port_t port)
+gearman_connection_st *gearman_connection_create(gearman_universal_st& universal,
+                                                 const char *host, const in_port_t& port)
 {
-  gearman_connection_st *connection= gearman_connection_create(universal, NULL);
+  gearman_connection_st *connection= __connection_create(universal);
   if (connection)
   {
     connection->set_host(host, port);
 
     if (gearman_failed(connection->lookup()))
     {
-      gearman_gerror(universal, GEARMAN_GETADDRINFO);
       delete connection;
       return NULL;
     }
@@ -220,7 +181,7 @@
 gearman_connection_st *gearman_connection_copy(gearman_universal_st& universal,
                                                const gearman_connection_st& from)
 {
-  gearman_connection_st *connection= gearman_connection_create(universal);
+  gearman_connection_st *connection= __connection_create(universal);
 
   if (connection)
   {
@@ -278,7 +239,7 @@
  * Public Definitions
  */
 
-void gearman_connection_st::set_host(const char *host_, const in_port_t& port_)
+void gearman_connection_st::set_host(const char *host_, const in_port_t port_)
 {
   if (port_ < 1)
   {
@@ -683,7 +644,7 @@
   if ((ret= getaddrinfo(_host, _service, &ai, &(_addrinfo))))
   {
     reset_addrinfo();
-    return gearman_universal_set_error(universal, GEARMAN_GETADDRINFO, GEARMAN_AT, "getaddrinfo:%s", gai_strerror(ret));
+    return gearman_universal_set_error(universal, GEARMAN_GETADDRINFO, GEARMAN_AT, "%s:%d getaddrinfo:%s", host(), service(), gai_strerror(ret));
   }
 
   addrinfo_next= _addrinfo;

=== modified file 'libgearman/connection.hpp'
--- libgearman/connection.hpp	2013-06-11 01:05:38 +0000
+++ libgearman/connection.hpp	2013-06-25 23:50:39 +0000
@@ -115,14 +115,23 @@
 
   void free_private_packet();
 
-  gearman_connection_st(gearman_universal_st &universal_arg,
-                        gearman_connection_options_t *options);
+  gearman_connection_st(gearman_universal_st &universal_arg);
 
   ~gearman_connection_st();
 
-  void set_host( const char *host, const in_port_t& port);
+  void set_host( const char *host, const in_port_t port);
   void set_host( const char *host, const char* service);
 
+  const char* host(void) const
+  {
+    return _host;
+  }
+
+  const char* service(void) const
+  {
+    return _service;
+  }
+
   gearman_return_t send_packet(const gearman_packet_st&, const bool flush_buffer);
   size_t send_and_flush(const void *data, size_t data_size, gearman_return_t *ret_ptr);
 
@@ -172,14 +181,6 @@
   gearman_packet_st *_recv_packet;
 };
 
-/**
- * Initialize a connection structure. Always check the return value even if
- * passing in a pre-allocated structure. Some other initialization may have
- * failed.
- */
-gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
-                                                 gearman_connection_options_t *options= NULL);
-
 gearman_connection_st *gearman_connection_copy(gearman_universal_st& universal,
                                                const gearman_connection_st& from);
 
@@ -194,8 +195,8 @@
  * @return On success, a pointer to the (possibly allocated) structure. On
  *  failure this will be NULL.
  */
-gearman_connection_st *gearman_connection_create_args(gearman_universal_st &universal,
-                                                      const char *host, in_port_t port);
+gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
+                                                 const char *host, const in_port_t&);
 
-gearman_connection_st *gearman_connection_create_args(gearman_universal_st &universal,
-                                                      const char* host, const char* service);
+gearman_connection_st *gearman_connection_create(gearman_universal_st &universal,
+                                                 const char* host, const char* service);

=== modified file 'libgearman/worker.cc'
--- libgearman/worker.cc	2013-06-11 01:05:38 +0000
+++ libgearman/worker.cc	2013-06-25 23:50:39 +0000
@@ -445,7 +445,7 @@
 {
   if (worker and worker->impl())
   {
-    if (gearman_connection_create_args(worker->impl()->universal, host, port) == NULL)
+    if (gearman_connection_create(worker->impl()->universal, host, port) == NULL)
     {
       return gearman_universal_error_code(worker->impl()->universal);
     }

=== modified file 'tests/libgearman-1.0/internals.cc'
--- tests/libgearman-1.0/internals.cc	2013-06-11 06:48:11 +0000
+++ tests/libgearman-1.0/internals.cc	2013-06-25 23:50:39 +0000
@@ -94,23 +94,23 @@
     gearman_universal_clone(destination, gear);
 
     { // Test all of the flags
-      test_truth(destination.options.non_blocking == gear.options.non_blocking);
+      ASSERT_EQ(destination.options.non_blocking, gear.options.non_blocking);
     }
-    test_truth(destination._namespace == gear._namespace);
-    test_truth(destination.verbose == gear.verbose);
-    test_truth(destination.con_count == gear.con_count);
-    test_truth(destination.packet_count == gear.packet_count);
-    test_truth(destination.pfds_size == gear.pfds_size);
-    test_truth(destination._error.last_errno == gear._error.last_errno);
-    test_truth(destination.timeout == gear.timeout);
-    test_truth(destination.con_list == gear.con_list);
-    test_truth(destination.packet_list == gear.packet_list);
-    test_truth(destination.pfds == gear.pfds);
-    test_truth(destination.log_fn == gear.log_fn);
-    test_truth(destination.log_context == gear.log_context);
-    test_truth(destination.allocator.malloc == gear.allocator.malloc);
-    test_truth(destination.allocator.context == gear.allocator.context);
-    test_truth(destination.allocator.free == gear.allocator.free);
+    ASSERT_EQ(destination._namespace,  gear._namespace);
+    ASSERT_EQ(destination.verbose,  gear.verbose);
+    ASSERT_EQ(destination.con_count,  gear.con_count);
+    ASSERT_EQ(destination.packet_count,  gear.packet_count);
+    ASSERT_EQ(destination.pfds_size,  gear.pfds_size);
+    ASSERT_EQ(destination._error.last_errno,  gear._error.last_errno);
+    ASSERT_EQ(destination.timeout,  gear.timeout);
+    ASSERT_EQ(destination.con_list,  gear.con_list);
+    ASSERT_EQ(destination.packet_list,  gear.packet_list);
+    ASSERT_EQ(destination.pfds,  gear.pfds);
+    ASSERT_EQ(destination.log_fn,  gear.log_fn);
+    ASSERT_EQ(destination.log_context,  gear.log_context);
+    ASSERT_EQ(destination.allocator.malloc,  gear.allocator.malloc);
+    ASSERT_EQ(destination.allocator.context,  gear.allocator.context);
+    ASSERT_EQ(destination.allocator.free,  gear.allocator.free);
 
     gearman_universal_free(gear);
   }
@@ -171,7 +171,7 @@
   gearman_universal_st universal(options);
 
   { // Initial Allocated, no changes
-    test_truth(universal.options.non_blocking);
+    ASSERT_TRUE(universal.options.non_blocking);
   }
   gearman_universal_free(universal);
 
@@ -200,11 +200,11 @@
   ASSERT_FALSE(universal._namespace);
 
   gearman_universal_set_namespace(universal, gearman_literal_param("my_dog"));
-  test_truth(universal._namespace);
+  ASSERT_TRUE(universal._namespace);
 
   gearman_universal_st clone;
   gearman_universal_clone(clone, universal);
-  test_truth(clone._namespace);
+  ASSERT_TRUE(clone._namespace);
 
   gearman_universal_free(universal);
   gearman_universal_free(clone);
@@ -218,7 +218,7 @@
   gearman_universal_st universal(options);
 
   { // Initial Allocated, no changes
-    test_truth(universal.options.non_blocking);
+    ASSERT_TRUE(universal.options.non_blocking);
   }
 
   ASSERT_TRUE(gearman_universal_is_non_blocking(universal));
@@ -259,8 +259,8 @@
 {
   gearman_universal_st universal;
 
-  gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL);
-  test_truth(connection_ptr);
+  gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL, (const char*)(GEARMAN_DEFAULT_TCP_PORT_STRING));
+  ASSERT_TRUE(connection_ptr);
 
   ASSERT_FALSE(connection_ptr->options.ready);
   ASSERT_FALSE(connection_ptr->options.packet_in_use);
@@ -274,8 +274,8 @@
 {
   gearman_universal_st universal;
 
-  gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL);
-  test_truth(connection_ptr);
+  gearman_connection_st *connection_ptr= gearman_connection_create(universal, NULL, (const char*)(GEARMAN_DEFAULT_TCP_PORT_STRING));
+  ASSERT_TRUE(connection_ptr);
 
   ASSERT_FALSE(connection_ptr->options.ready);
   ASSERT_FALSE(connection_ptr->options.packet_in_use);
@@ -305,7 +305,7 @@
   ASSERT_FALSE(packet.options.complete);
   ASSERT_FALSE(packet.options.free_data);
 
-  test_truth(packet_ptr == &packet);
+  ASSERT_EQ(packet_ptr, &packet);
 
   gearman_packet_free(packet_ptr);
   ASSERT_FALSE(packet.options.is_allocated);
@@ -326,13 +326,13 @@
 
   gearman_packet_st packet;
 
-  test_truth(gearman_packet_create(universal, packet));
+  ASSERT_TRUE(gearman_packet_create(universal, packet));
 
   gearman_packet_give_data(packet, data, data_size);
 
-  test_truth(packet.data == data);
+  ASSERT_EQ(packet.data, data);
   ASSERT_EQ(packet.data_size, data_size);
-  test_truth(packet.options.free_data);
+  ASSERT_TRUE(packet.options.free_data);
 
   gearman_packet_free(&packet);
   gearman_universal_free(universal);
@@ -354,13 +354,13 @@
   gearman_packet_st packet;
 
   gearman_packet_st *packet_ptr= gearman_packet_create(universal, packet);
-  test_truth(packet_ptr);
+  ASSERT_TRUE(packet_ptr);
 
   gearman_packet_give_data(packet, data, data_size);
 
-  test_truth(packet_ptr->data == data);
+  ASSERT_EQ(packet_ptr->data, data);
   ASSERT_EQ(data_size, packet_ptr->data_size);
-  test_truth(packet_ptr->options.free_data);
+  ASSERT_TRUE(packet_ptr->options.free_data);
 
   size_t mine_size;
   char *mine= (char *)gearman_packet_take_data(packet, &mine_size);

=== modified file 'tests/libgearman-1.0/regression.cc'
--- tests/libgearman-1.0/regression.cc	2013-01-03 02:58:19 +0000
+++ tests/libgearman-1.0/regression.cc	2013-06-25 23:50:39 +0000
@@ -126,9 +126,7 @@
   {
     gearman_packet_st packet;
     gearman_connection_st *con_ptr;
-    test_truth(con_ptr= gearman_connection_create(universal, NULL));
-
-    con_ptr->set_host(NULL, default_port());
+    test_truth(con_ptr= gearman_connection_create(universal, NULL, default_port()));
 
     args[0]= "testUnregisterFunction";
     args_size[0]= strlen("testUnregisterFunction");
@@ -159,9 +157,7 @@
 
     delete con_ptr;
 
-    test_truth(con_ptr= gearman_connection_create(universal, NULL));
-
-    con_ptr->set_host(NULL, default_port());
+    test_truth(con_ptr= gearman_connection_create(universal, NULL, default_port()));
 
     args[0]= "testUnregisterFunction";
     args_size[0]= strlen("testUnregisterFunction");

=== modified file 'tests/libgearman-1.0/worker_test.cc'
--- tests/libgearman-1.0/worker_test.cc	2013-06-15 01:53:12 +0000
+++ tests/libgearman-1.0/worker_test.cc	2013-06-25 23:50:39 +0000
@@ -888,9 +888,7 @@
   gearman_universal_st universal;
 
   gearman_connection_st *connection1;
-  test_truth(connection1= gearman_connection_create(universal, NULL));
-
-  connection1->set_host(NULL, libtest::default_port());
+  test_truth(connection1= gearman_connection_create(universal, NULL, default_port()));
 
   gearman_packet_st packet;
   args[0]= "abandoned_worker";
@@ -921,9 +919,7 @@
   gearman_packet_free(&packet);
 
   gearman_connection_st *connection2;
-  test_truth(connection2= gearman_connection_create(universal, NULL));
-
-  connection2->set_host(NULL, libtest::default_port());
+  test_truth(connection2= gearman_connection_create(universal, NULL, default_port()));
 
   args[0]= "abandoned_worker";
   args_size[0]= strlen("abandoned_worker");