Add log testing.

ChangeLog

@@ -1,3 +1,6 @@
+1.1.8
+*
+
 1.1.7 Mon May  6 06:46:20 EDT 2013
 * Cleanup of error codes returned by gearmand.
 * gearmand will now set its port from the env variable GEARMAND_PORT.

configure.ac

@@ -6,9 +6,10 @@
 # Use and distribution licensed under the BSD license.  See
 # the COPYING file in this directory for full text.
 
+m4_include([version.m4])
 AC_REVISION([m4_esyscmd_s([bzr revno])])
 AC_PREREQ([2.63])
-AC_INIT([gearmand],[1.1.7],[https://bugs.launchpad.net/gearmand],[gearmand],[http://gearman.info/])
+AC_INIT([gearmand],VERSION_NUMBER,[https://bugs.launchpad.net/gearmand],[gearmand],[http://gearman.info/])
 AC_CONFIG_AUX_DIR([build-aux])
 AC_CONFIG_MACRO_DIR([m4])
 

libgearman/error.cc

@@ -41,6 +41,8 @@
 
 #include "libgearman/assert.hpp"
 
+#include "libgearman/log.hpp"
+
 #include <cerrno>
 #include <cstdarg>
 #include <cstdio>
@@ -120,12 +122,8 @@ gearman_return_t gearman_universal_set_error(gearman_universal_st& universal,
     universal._error.last_error[GEARMAN_MAX_ERROR_SIZE -1]= 0;
   }
 
-  if (universal.log_fn)
-  {
-    universal.log_fn(universal._error.last_error,
-                     universal._error.rc == GEARMAN_MEMORY_ALLOCATION_FAILURE ? GEARMAN_VERBOSE_FATAL : GEARMAN_VERBOSE_ERROR,
-                     static_cast<void *>(universal.log_context));
-  }
+  gearman_log_error(universal,
+                    universal._error.rc == GEARMAN_MEMORY_ALLOCATION_FAILURE ? GEARMAN_VERBOSE_FATAL : GEARMAN_VERBOSE_ERROR);
 
   return universal._error.rc;
 }
@@ -152,12 +150,8 @@ gearman_return_t gearman_universal_set_gerror(gearman_universal_st& universal,
     return GEARMAN_ARGUMENT_TOO_LARGE;
   }
 
-  if (universal.log_fn)
-  {
-    universal.log_fn(universal._error.last_error,
-                     universal._error.rc == GEARMAN_MEMORY_ALLOCATION_FAILURE ? GEARMAN_VERBOSE_FATAL : GEARMAN_VERBOSE_ERROR,
-                     static_cast<void *>(universal.log_context));
-  }
+  gearman_log_error(universal,
+                    universal._error.rc == GEARMAN_MEMORY_ALLOCATION_FAILURE ? GEARMAN_VERBOSE_FATAL : GEARMAN_VERBOSE_ERROR);
 
   return rc;
 }
@@ -213,12 +207,8 @@ gearman_return_t gearman_universal_set_perror(gearman_universal_st &universal,
     universal._error.last_error[GEARMAN_MAX_ERROR_SIZE -1]= 0;
   }
 
-  if (universal.log_fn)
-  {
-    universal.log_fn(universal._error.last_error, 
-                     universal._error.rc == GEARMAN_MEMORY_ALLOCATION_FAILURE ? GEARMAN_VERBOSE_FATAL : GEARMAN_VERBOSE_ERROR,
-                     static_cast<void *>(universal.log_context));
-  }
+  gearman_log_error(universal,
+                    universal._error.rc == GEARMAN_MEMORY_ALLOCATION_FAILURE ? GEARMAN_VERBOSE_FATAL : GEARMAN_VERBOSE_ERROR);
 
   return universal._error.rc;
 }

libgearman/log.cc

@@ -48,8 +48,8 @@
 #include <cstdio>
 
 #pragma GCC diagnostic ignored "-Wformat-nonliteral"
-void gearman_log(gearman_universal_st& state, gearman_verbose_t verbose,
-                 const char *format, va_list args)
+static void __logger(gearman_universal_st& state, gearman_verbose_t verbose,
+                     const char *format, va_list args)
 {
   if (state.log_fn)
   {
@@ -59,6 +59,29 @@ void gearman_log(gearman_universal_st& state, gearman_verbose_t verbose,
   }
 }
 
+void gearman_log(gearman_universal_st& state, gearman_verbose_t verbose,
+                 const char *format, ...)
+{
+  va_list args;
+
+  if (state.verbose >= verbose)
+  {
+    va_start(args, format);
+    __logger(state, verbose, format, args);
+    va_end(args);
+  }
+}
+
+void gearman_log_error(gearman_universal_st& state, gearman_verbose_t verbose)
+{
+  if (state.verbose >= verbose)
+  {
+    if (state.log_fn)
+    {
+      state.log_fn(state.error(), verbose, state.log_context);
+    }
+  }
+}
 
 void gearman_log_info(gearman_universal_st& gearman, const char *format, ...)
 {
@@ -67,7 +90,7 @@ void gearman_log_info(gearman_universal_st& gearman, const char *format, ...)
   if (gearman.verbose >= GEARMAN_VERBOSE_INFO)
   {
     va_start(args, format);
-    gearman_log(gearman, GEARMAN_VERBOSE_INFO, format, args);
+    __logger(gearman, GEARMAN_VERBOSE_INFO, format, args);
     va_end(args);
   }
 }
@@ -79,7 +102,7 @@ void gearman_log_debug(gearman_universal_st& gearman, const char *format, ...)
   if (gearman.verbose >= GEARMAN_VERBOSE_DEBUG)
   {
     va_start(args, format);
-    gearman_log(gearman, GEARMAN_VERBOSE_DEBUG, format, args);
+    __logger(gearman, GEARMAN_VERBOSE_DEBUG, format, args);
     va_end(args);
   }
 }

libgearman/log.hpp

@@ -60,7 +60,7 @@
  * @param[in] args Variable argument list that has been initialized.
  */
 void gearman_log(gearman_universal_st& gearman, gearman_verbose_t verbose,
-                 const char *format, va_list args);
+                 const char *format, ...);
 
 /**
  * Log an info message, see gearman_log() for argument details.
@@ -71,3 +71,5 @@ void gearman_log_info(gearman_universal_st& gearman, const char *format, ...);
  * Log a debug message, see gearman_log() for argument details.
  */
 void gearman_log_debug(gearman_universal_st& gearman, const char *format, ...);
+
+void gearman_log_error(gearman_universal_st& state, gearman_verbose_t verbose);

libgearman/worker.cc

@@ -166,7 +166,7 @@ gearman_worker_st *gearman_worker_clone(gearman_worker_st *worker,
 
 void gearman_worker_free(gearman_worker_st *worker)
 {
-  if (worker)
+  if (worker and worker->impl())
   {
     if (worker->impl()->universal.wakeup_fd[0] != INVALID_SOCKET)
     {
@@ -209,7 +209,7 @@ void gearman_worker_free(gearman_worker_st *worker)
 
 const char *gearman_worker_error(const gearman_worker_st *worker_shell)
 {
-  if (worker_shell)
+  if (worker_shell and worker_shell->impl())
   {
     return worker_shell->impl()->universal.error();
   }
@@ -219,7 +219,7 @@ const char *gearman_worker_error(const gearman_worker_st *worker_shell)
 
 int gearman_worker_errno(gearman_worker_st *worker_shell)
 {
-  if (worker_shell)
+  if (worker_shell and worker_shell->impl())
   {
     return worker_shell->impl()->universal.last_errno();
   }
@@ -261,31 +261,26 @@ gearman_worker_options_t gearman_worker_options(const gearman_worker_st *worker)
 void gearman_worker_set_options(gearman_worker_st *worker,
                                 gearman_worker_options_t options)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return;
-  }
-
-  gearman_worker_options_t usable_options[]= {
-    GEARMAN_WORKER_NON_BLOCKING,
-    GEARMAN_WORKER_GRAB_UNIQ,
-    GEARMAN_WORKER_GRAB_ALL,
-    GEARMAN_WORKER_TIMEOUT_RETURN,
-    GEARMAN_WORKER_MAX
-  };
-
-  gearman_worker_options_t *ptr;
+    gearman_worker_options_t usable_options[]= {
+      GEARMAN_WORKER_NON_BLOCKING,
+      GEARMAN_WORKER_GRAB_UNIQ,
+      GEARMAN_WORKER_GRAB_ALL,
+      GEARMAN_WORKER_TIMEOUT_RETURN,
+      GEARMAN_WORKER_MAX
+    };
 
-
-  for (ptr= usable_options; *ptr != GEARMAN_WORKER_MAX ; ptr++)
-  {
-    if (options & *ptr)
+    for (gearman_worker_options_t* ptr= usable_options; *ptr != GEARMAN_WORKER_MAX ; ++ptr)
     {
-      gearman_worker_add_options(worker, *ptr);
-    }
-    else
-    {
-      gearman_worker_remove_options(worker, *ptr);
+      if (options & *ptr)
+      {
+        gearman_worker_add_options(worker, *ptr);
+      }
+      else
+      {
+        gearman_worker_remove_options(worker, *ptr);
+      }
     }
   }
 }
@@ -293,146 +288,137 @@ void gearman_worker_set_options(gearman_worker_st *worker,
 void gearman_worker_add_options(gearman_worker_st *worker,
                                 gearman_worker_options_t options)
 {
-  if (worker == NULL)
-  {
-    return;
-  }
-
-  if (options & GEARMAN_WORKER_NON_BLOCKING)
+  if (worker and worker->impl())
   {
-    gearman_universal_add_options(worker->impl()->universal, GEARMAN_UNIVERSAL_NON_BLOCKING);
-    worker->impl()->options.non_blocking= true;
-  }
+    if (options & GEARMAN_WORKER_NON_BLOCKING)
+    {
+      gearman_universal_add_options(worker->impl()->universal, GEARMAN_UNIVERSAL_NON_BLOCKING);
+      worker->impl()->options.non_blocking= true;
+    }
 
-  if (options & GEARMAN_WORKER_GRAB_UNIQ)
-  {
-    worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB_UNIQ;
-    gearman_return_t rc= gearman_packet_pack_header(&(worker->impl()->grab_job));
-    (void)(rc);
-    assert(gearman_success(rc));
-    worker->impl()->options.grab_uniq= true;
-  }
+    if (options & GEARMAN_WORKER_GRAB_UNIQ)
+    {
+      worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB_UNIQ;
+      gearman_return_t rc= gearman_packet_pack_header(&(worker->impl()->grab_job));
+      (void)(rc);
+      assert(gearman_success(rc));
+      worker->impl()->options.grab_uniq= true;
+    }
 
-  if (options & GEARMAN_WORKER_GRAB_ALL)
-  {
-    worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB_ALL;
-    gearman_return_t rc= gearman_packet_pack_header(&(worker->impl()->grab_job));
-    (void)(rc);
-    assert(gearman_success(rc));
-    worker->impl()->options.grab_all= true;
-  }
+    if (options & GEARMAN_WORKER_GRAB_ALL)
+    {
+      worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB_ALL;
+      gearman_return_t rc= gearman_packet_pack_header(&(worker->impl()->grab_job));
+      (void)(rc);
+      assert(gearman_success(rc));
+      worker->impl()->options.grab_all= true;
+    }
 
-  if (options & GEARMAN_WORKER_TIMEOUT_RETURN)
-  {
-    worker->impl()->options.timeout_return= true;
+    if (options & GEARMAN_WORKER_TIMEOUT_RETURN)
+    {
+      worker->impl()->options.timeout_return= true;
+    }
   }
 }
 
 void gearman_worker_remove_options(gearman_worker_st *worker,
                                    gearman_worker_options_t options)
 {
-  if (worker == NULL)
-  {
-    return;
-  }
-
-  if (options & GEARMAN_WORKER_NON_BLOCKING)
+  if (worker and worker->impl())
   {
-    gearman_universal_remove_options(worker->impl()->universal, GEARMAN_UNIVERSAL_NON_BLOCKING);
-    worker->impl()->options.non_blocking= false;
-  }
+    if (options & GEARMAN_WORKER_NON_BLOCKING)
+    {
+      gearman_universal_remove_options(worker->impl()->universal, GEARMAN_UNIVERSAL_NON_BLOCKING);
+      worker->impl()->options.non_blocking= false;
+    }
 
-  if (options & GEARMAN_WORKER_TIMEOUT_RETURN)
-  {
-    worker->impl()->options.timeout_return= false;
-    gearman_universal_set_timeout(worker->impl()->universal, GEARMAN_WORKER_WAIT_TIMEOUT);
-  }
+    if (options & GEARMAN_WORKER_TIMEOUT_RETURN)
+    {
+      worker->impl()->options.timeout_return= false;
+      gearman_universal_set_timeout(worker->impl()->universal, GEARMAN_WORKER_WAIT_TIMEOUT);
+    }
 
-  if (options & GEARMAN_WORKER_GRAB_UNIQ)
-  {
-    worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB;
-    (void)gearman_packet_pack_header(&(worker->impl()->grab_job));
-    worker->impl()->options.grab_uniq= false;
-  }
+    if (options & GEARMAN_WORKER_GRAB_UNIQ)
+    {
+      worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB;
+      (void)gearman_packet_pack_header(&(worker->impl()->grab_job));
+      worker->impl()->options.grab_uniq= false;
+    }
 
-  if (options & GEARMAN_WORKER_GRAB_ALL)
-  {
-    worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB;
-    (void)gearman_packet_pack_header(&(worker->impl()->grab_job));
-    worker->impl()->options.grab_all= false;
+    if (options & GEARMAN_WORKER_GRAB_ALL)
+    {
+      worker->impl()->grab_job.command= GEARMAN_COMMAND_GRAB_JOB;
+      (void)gearman_packet_pack_header(&(worker->impl()->grab_job));
+      worker->impl()->options.grab_all= false;
+    }
   }
 }
 
 int gearman_worker_timeout(gearman_worker_st *worker)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return 0;
+    return gearman_universal_timeout(worker->impl()->universal);
   }
 
-  return gearman_universal_timeout(worker->impl()->universal);
+  return 0;
 }
 
 void gearman_worker_set_timeout(gearman_worker_st *worker, int timeout)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return;
+    gearman_worker_add_options(worker, GEARMAN_WORKER_TIMEOUT_RETURN);
+    gearman_universal_set_timeout(worker->impl()->universal, timeout);
   }
-
-  gearman_worker_add_options(worker, GEARMAN_WORKER_TIMEOUT_RETURN);
-  gearman_universal_set_timeout(worker->impl()->universal, timeout);
 }
 
 void *gearman_worker_context(const gearman_worker_st *worker)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return NULL;
+    return worker->impl()->context;
   }
 
-  return worker->impl()->context;
+  return NULL;
 }
 
 void gearman_worker_set_context(gearman_worker_st *worker, void *context)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return;
+    worker->impl()->context= context;
   }
-
-  worker->impl()->context= context;
 }
 
 void gearman_worker_set_log_fn(gearman_worker_st *worker,
                                gearman_log_fn *function, void *context,
                                gearman_verbose_t verbose)
 {
-  gearman_set_log_fn(worker->impl()->universal, function, context, verbose);
+  if (worker and worker->impl())
+  {
+    gearman_set_log_fn(worker->impl()->universal, function, context, verbose);
+  }
 }
 
 void gearman_worker_set_workload_malloc_fn(gearman_worker_st *worker,
                                            gearman_malloc_fn *function,
                                            void *context)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return;
+    gearman_set_workload_malloc_fn(worker->impl()->universal, function, context);
   }
-
-  gearman_set_workload_malloc_fn(worker->impl()->universal, function, context);
 }
 
 void gearman_worker_set_workload_free_fn(gearman_worker_st *worker,
                                          gearman_free_fn *function,
                                          void *context)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return;
+    gearman_set_workload_free_fn(worker->impl()->universal, function, context);
   }
-
-  gearman_set_workload_free_fn(worker->impl()->universal, function, context);
 }
 
 gearman_return_t gearman_worker_add_server(gearman_worker_st *worker,
@@ -458,12 +444,10 @@ gearman_return_t gearman_worker_add_servers(gearman_worker_st *worker, const cha
 
 void gearman_worker_remove_servers(gearman_worker_st *worker)
 {
-  if (worker == NULL)
+  if (worker and worker->impl())
   {
-    return;
+    gearman_free_all_cons(worker->impl()->universal);
   }
-
-  gearman_free_all_cons(worker->impl()->universal);
 }
 
 gearman_return_t gearman_worker_wait(gearman_worker_st *worker)

tests/libgearman-1.0/worker_test.cc

@@ -175,6 +175,13 @@ static test_return_t gearman_worker_options_TEST(void *)
   return TEST_SUCCESS;
 }
 
+static test_return_t gearman_worker_set_log_fn_TEST(void *)
+{
+  gearman_worker_set_log_fn(NULL, NULL, NULL, GEARMAN_VERBOSE_MAX);
+
+  return TEST_SUCCESS;
+}
+
 static test_return_t option_test(void *)
 {
   gearman_worker_options_t default_options;
@@ -799,6 +806,13 @@ static test_return_t gearman_worker_add_function_test(void *)
   return TEST_SUCCESS;
 }
 
+static void log_callback(const char *, gearman_verbose_t, void *context)
+{
+  uint32_t *counter= (uint32_t*)context;
+
+  *counter= *counter +1;
+}
+
 static test_return_t gearman_worker_timeout_TEST(void *)
 {
   libgearman::Worker worker(libtest::default_port());
@@ -808,8 +822,12 @@ static test_return_t gearman_worker_timeout_TEST(void *)
 
   gearman_worker_set_timeout(&worker, 1000);
 
+  uint32_t counter= 0;
+  gearman_worker_set_log_fn(&worker, log_callback, &counter, GEARMAN_VERBOSE_ERROR);
+
   gearman_return_t ret= gearman_worker_work(&worker);
   ASSERT_EQ(ret, GEARMAN_TIMEOUT);
+  ASSERT_EQ(counter, 1);
 
   return TEST_SUCCESS;
 }
@@ -1292,6 +1310,7 @@ test_st gearman_worker_st_NULL_invocation_TESTS[] ={
   {"gearman_worker_errno()", 0, gearman_worker_errno_TEST },
   {"gearman_worker_errno() no error", 0, gearman_worker_errno_no_error_TEST },
   {"gearman_worker_options()", 0, gearman_worker_options_TEST },
+  {"gearman_worker_set_log_fn()", 0, gearman_worker_set_log_fn_TEST },
   {0, 0, 0}
 };
 

version.m4

@@ -0,0 +1 @@
+m4_define([VERSION_NUMBER], [1.1.8])