* refactor backend modules for new library interface (closes #10)

src/backends/backend.c

@@ -54,3 +54,33 @@ usage_dec(const char *id) {
   /* not found or list is empty */
   return 0;
 }
+
+static void
+logcb_stub(enum loglevel lvl, const char *str) {
+  assert(str != NULL);
+  (void)(lvl);
+  (void)(str);
+}
+
+__attribute__ ((format (printf, 3, 4)))
+static void
+log_msg(const cfg_t *cfg, enum loglevel lvl, const char *format, ...) {
+  char buf[4096] = "";
+  va_list args;
+  size_t len;
+
+  len = snprintf(buf, sizeof(buf), "backend/%s ", MODNAME);
+  va_start(args, format);
+  vsnprintf(buf + len, sizeof(buf) - len, format, args);
+  va_end(args);
+
+  cfg->logcb(lvl, buf);
+}
+
+void
+logcb(cfg_t *cfg, void (*cb)(enum loglevel lvl, const char *msg)) {
+  assert(cfg != NULL);
+  assert(cb  != NULL);
+
+  cfg->logcb = cb;
+}

src/backends/backend.h

@@ -5,6 +5,16 @@
  * published by the Free Software Foundation.
  */
 #include <stdbool.h>
+#include <stdarg.h>
+
+enum loglevel {
+  debug  = 0,
+  info   = 1,
+  notice = 2,
+  warn   = 3,
+  error  = 4,
+  fatal  = 5,
+}; /* see log.h */
 
 /**
  * @file
@@ -22,9 +32,8 @@
  *   f2b =>  backend [label="config(cfg, param, value)"];
  *   f2b <<  backend [label="true"];
  *   f2b =>  backend [label="config(cfg, param, value)"];
+ *   f2b <<= backend [label="logcb(level, char *msg)"];
  *   f2b <<  backend [label="false"];
- *   f2b =>  backend [label="error(cfg)"];
- *   f2b <<  backend [label="const char *error"];
  *       |||;
  *   f2b =>  backend [label="ready(cfg)"];
  *   f2b <<  backend [label="true"];
@@ -40,9 +49,8 @@
  *   f2b =>  backend [label="ban(cfg, ip)"];
  *   f2b <<  backend [label="true"];
  *   f2b =>  backend [label="ban(cfg, ip)"];
+ *   f2b <<= backend [label="logcb(level, char *msg)"];
  *   f2b <<  backend [label="false"];
- *   f2b =>  backend [label="error(cfg)"];
- *   f2b <<  backend [label="const char *error"];
  *       ...         [label="time passed"];
  *   f2b =>  backend [label="ping(cfg)"];
  *   f2b <<  backend [label="true"];
@@ -89,7 +97,7 @@ extern cfg_t *create(const char *id);
  * @param cfg Module handler
  * @param key Parameter name
  * @param value Parameter value
- * @returns true on success, false on error with setting intenal error buffer
+ * @returns true on success, false on error
  */
 extern bool   config(cfg_t *cfg, const char *key, const char *value);
 /**
@@ -100,18 +108,10 @@ extern bool   config(cfg_t *cfg, const char *key, const char *value);
  * at least should handle 'shared' option
  */
 extern bool    ready(cfg_t *cfg);
-/**
- * @brief Returns last error description
- * @param cfg Module handler
- * @returns Pointer to string with description of last error
- * @note Returned pointer not marked with const, because libdl complains,
- *       but contents on pointer should not be modified or written in any way
- */
-extern char   *error(cfg_t *cfg);
 /**
  * @brief Allocate resources and start processing
  * @param cfg Module handler
- * @returns true on success, false on error with setting intenal error buffer
+ * @returns true on success, false on error
  */
 extern bool    start(cfg_t *cfg);
 /**
@@ -126,18 +126,25 @@ extern bool     stop(cfg_t *cfg);
  * @returns true on success
  */
 extern bool     ping(cfg_t *cfg);
+/**
+ * @brief Sets the log callback
+ * @param cfg Module handler
+ * @param cb Logging callback
+ * @note Optional, if this function is not called, warnings/errors of module will be suppressed
+ */
+extern void    logcb(cfg_t *cfg, void (*cb)(enum loglevel l, const char *msg));
 /**
  * @brief Make a rabbit disappear
  * @param cfg Module handler
  * @param ip IP address
- * @returns true on success, false on error with setting intenal error buffer
+ * @returns true on success, false on error
  */
 extern bool      ban(cfg_t *cfg, const char *ip);
 /**
  * @brief Check is some ip already banned
  * @param cfg Module handler
  * @param ip IP address
- * @returns true on success, false on error with setting intenal error buffer
+ * @returns true on success, false on error
  * @note If this action is meaningless for backend it should return true
  */
 extern bool    check(cfg_t *cfg, const char *ip);
@@ -145,7 +152,7 @@ extern bool    check(cfg_t *cfg, const char *ip);
  * @brief Make a rabbit appear again
  * @param cfg Module handler
  * @param ip IP address
- * @returns true on success, false on error with setting intenal error buffer
+ * @returns true on success, false on error
  * @note If this action is meaningless for backend it should return true
  */
 extern bool    unban(cfg_t *cfg, const char *ip);

src/backends/exec.c

@@ -16,10 +16,11 @@
 #include "../strlcpy.h"
 
 #include "backend.h"
-#include "shared.c"
+#define MODNAME "exec"
 
 typedef struct cmd_t {
   struct cmd_t *next;
+  char  *orig;   /**< stores original command, used in log messages */
   char  *args;   /**< stores path of cmd & args, delimited by '\0' */
   char **argv;   /**< stores array of pointers to args + NULL */
   size_t argc;   /**< args count */
@@ -29,7 +30,7 @@ typedef struct cmd_t {
 
 struct _config {
   char name[ID_MAX + 1];
-  char error[256];
+  void (*logcb)(enum loglevel lvl, const char *msg);
   time_t timeout;
   bool  shared;
   cmd_t *start;
@@ -39,6 +40,8 @@ struct _config {
   cmd_t *check;
 };
 
+#include "backend.c"
+
 static cmd_t *
 cmd_from_str(const char *str) {
   cmd_t *cmd = NULL;
@@ -50,6 +53,8 @@ cmd_from_str(const char *str) {
   if ((cmd = calloc(1, sizeof(cmd_t))) == NULL)
     return NULL;
 
+  if ((cmd->orig = strdup(str)) == NULL)
+    goto cleanup;
   if ((cmd->args = strdup(str)) == NULL)
     goto cleanup;
 
@@ -77,6 +82,7 @@ cmd_from_str(const char *str) {
   return cmd;
 
   cleanup:
+  free(cmd->orig);
   free(cmd->args);
   free(cmd->argv);
   free(cmd);
@@ -104,6 +110,7 @@ cmd_list_destroy(cmd_t *list) {
 
   for (; list != NULL; list = next) {
     next = list->next;
+    free(list->orig);
     free(list->args);
     free(list->argv);
     free(list);
@@ -132,17 +139,14 @@ cmd_list_exec(cfg_t *cfg, cmd_t *list, const char *ip) {
       if (WIFEXITED(status) && WEXITSTATUS(status) == 0)
         continue;
       if (WIFEXITED(status)) {
-        snprintf(cfg->error, sizeof(cfg->error),
+        log_msg(cfg, error, "cmd '%s' terminated with code %d", cmd->orig, WEXITSTATUS(status));
-          "cmd '%s' terminated with code %d",
-          cmd->args, WEXITSTATUS(status));
       } else if (WIFSIGNALED(status)) {
-        snprintf(cfg->error, sizeof(cfg->error),
+        log_msg(cfg, error, "cmd '%s' terminated by signal %d", cmd->orig, WTERMSIG(status));
-          "cmd '%s' terminated by signal %d", cmd->args, WTERMSIG(status));
       }
       return false;
     } else {
       /* parent, fork error */
-      snprintf(cfg->error, sizeof(cfg->error), "can't fork(): %s", strerror(errno));
+      log_msg(cfg, error, "can't fork(): %s", strerror(errno));
       return false;
     }
   }
@@ -161,6 +165,7 @@ create(const char *id) {
     return NULL;
   strlcpy(cfg->name, id, sizeof(cfg->name));
 
+  cfg->logcb = &logcb_stub;
   return cfg;
 }
 
@@ -210,13 +215,6 @@ ready(cfg_t *cfg) {
   return false;
 }
 
-char *
-error(cfg_t *cfg) {
-  assert(cfg != NULL);
-
-  return cfg->error;
-}
-
 bool
 start(cfg_t *cfg) {
   assert(cfg != NULL);

src/backends/ipset.c

@@ -19,20 +19,25 @@
 #include "../strlcpy.h"
 
 #include "backend.h"
-#include "shared.c"
+#define MODNAME "ipset"
 
 struct _config {
   char name[ID_MAX + 1];
-  char error[256];
+  void (*logcb)(enum loglevel lvl, const char *msg);
   bool shared;
   struct ipset_session *sess;
 };
 
+#include "backend.c"
+
 inline static bool
-my_ipset_error(cfg_t *cfg) {
+my_ipset_error(cfg_t *cfg, const char *func) {
   struct ipset_data *data = NULL;
-  snprintf(cfg->error, sizeof(cfg->error), "ipset: %s",
+  if (ipset_session_error(cfg->sess)) {
-    ipset_session_error(cfg->sess));
+    log_msg(cfg, error, "%s: %s", func, ipset_session_error(cfg->sess));
+  } else /* IPSET_WARNING */ {
+    log_msg(cfg, warn, "%s: %s", func, ipset_session_warning(cfg->sess));
+  }
 
   ipset_session_report_reset(cfg->sess);
 
@@ -47,16 +52,19 @@ my_ipset_cmd(cfg_t *cfg, enum ipset_cmd cmd, const char *ip) {
   const struct ipset_type *type = NULL;
 
   if (ipset_parse_setname(cfg->sess, IPSET_SETNAME, cfg->name) < 0)
-    return my_ipset_error(cfg);
+    return my_ipset_error(cfg, "ipset_parse_setname())");
 
   if ((type = ipset_type_get(cfg->sess, cmd)) == NULL)
-    return my_ipset_error(cfg);
+    return my_ipset_error(cfg, "ipset_type_get()");
 
   if (ipset_parse_elem(cfg->sess, type->last_elem_optional, ip) < 0)
-    return my_ipset_error(cfg);
+    return my_ipset_error(cfg, "ipset_parse_elem()");
 
-  if (ipset_cmd(cfg->sess, cmd, 0) < 0)
+  if (ipset_cmd(cfg->sess, cmd, 0) < 0) {
-    return my_ipset_error(cfg);
+    if (cmd == IPSET_CMD_TEST && ipset_session_error(cfg->sess) == NULL)
+      return false; /* "ip is NOT in list" */
+    return my_ipset_error(cfg, "ipset_cmd()");
+  }
 
   return true;
 }
@@ -71,6 +79,7 @@ create(const char *id) {
     return NULL;
   strlcpy(cfg->name, id, sizeof(cfg->name));
 
+  cfg->logcb = &logcb_stub;
   return cfg;
 }
 
@@ -97,13 +106,6 @@ ready(cfg_t *cfg) {
   return false;
 }
 
-char *
-error(cfg_t *cfg) {
-  assert(cfg != NULL);
-
-  return cfg->error;
-}
-
 bool
 start(cfg_t *cfg) {
   assert(cfg != NULL);
@@ -114,15 +116,15 @@ start(cfg_t *cfg) {
   ipset_load_types();
 
   if ((cfg->sess = ipset_session_init(NULL)) == NULL) {
-    strlcpy(cfg->error, "can't init ipset session", sizeof(cfg->error));
+    log_msg(cfg, error, "can't init ipset session");
     return false;
   }
 
   if (ipset_session_output(cfg->sess, IPSET_LIST_NONE) < 0)
-    return my_ipset_error(cfg);
+    return my_ipset_error(cfg, "ipset_session_output()");
 
   if (ipset_envopt_parse(cfg->sess, IPSET_ENV_EXIST, NULL) < 0)
-    return my_ipset_error(cfg);
+    return my_ipset_error(cfg, "ipset_envopt_parse()");
 
   return true;
 }

src/backends/redis.c

@@ -20,12 +20,12 @@
 #include "../strlcpy.h"
 
 #include "backend.h"
-#include "shared.c"
+#define MODNAME "redis"
 
 struct _config {
   char name[ID_MAX + 1];
   char hash[ID_MAX * 2];
-  char error[256];
+  void (*logcb)(enum loglevel lvl, const char *msg);
   bool shared;
   time_t timeout;
   uint8_t ping_num; /*< current number of ping() call */
@@ -37,6 +37,8 @@ struct _config {
   redisContext *conn;
 };
 
+#include "backend.c"
+
 static bool
 redis_connect(cfg_t *cfg) {
   assert(cfg != NULL);
@@ -52,14 +54,14 @@ redis_connect(cfg_t *cfg) {
     if (!conn)
       break;
     if (conn->err) {
-      snprintf(cfg->error, sizeof(cfg->error), "Connection error: %s", conn->errstr);
+      log_msg(cfg, error, "Connection error: %s", conn->errstr);
       break;
     }
     if (cfg->password[0]) {
       if ((reply = redisCommand(conn, "AUTH %s", cfg->password)) == NULL)
         break;
       if (reply->type == REDIS_REPLY_ERROR) {
-        snprintf(cfg->error, sizeof(cfg->error), "auth error: %s", reply->str);
+        log_msg(cfg, error, "auth error: %s", reply->str);
         break;
       }
       freeReplyObject(reply);
@@ -68,7 +70,7 @@ redis_connect(cfg_t *cfg) {
       if ((reply = redisCommand(conn, "SELECT %d", cfg->database)) == NULL)
         break;
       if (reply->type == REDIS_REPLY_ERROR) {
-        snprintf(cfg->error, sizeof(cfg->error), "auth error: %s", reply->str);
+        log_msg(cfg, error, "auth error: %s", reply->str);
         break;
       }
       freeReplyObject(reply);
@@ -110,6 +112,7 @@ create(const char *id) {
   strlcpy(cfg->hash, "f2b-banned-", sizeof(cfg->hash));
   strlcat(cfg->hash, id, sizeof(cfg->hash));
 
+  cfg->logcb = &logcb_stub;
   return cfg;
 }
 
@@ -161,13 +164,6 @@ ready(cfg_t *cfg) {
   return false;
 }
 
-char *
-error(cfg_t *cfg) {
-  assert(cfg != NULL);
-
-  return cfg->error;
-}
-
 bool
 start(cfg_t *cfg) {
   assert(cfg != NULL);
@@ -202,14 +198,14 @@ ban(cfg_t *cfg, const char *ip) {
     if ((reply = redisCommand(cfg->conn, "HINCRBY %s %s %d", cfg->hash, ip, 1)) == NULL)
       break;
     if (reply->type == REDIS_REPLY_ERROR) {
-      snprintf(cfg->error, sizeof(cfg->error), "HINCRBY: %s", reply->str);
+      log_msg(cfg, error, "HINCRBY: %s", reply->str);
       break;
     }
     freeReplyObject(reply);
     if ((reply = redisCommand(cfg->conn, "PUBLISH %s %s",    cfg->hash, ip)) == NULL)
       break;
     if (reply->type == REDIS_REPLY_ERROR) {
-      snprintf(cfg->error, sizeof(cfg->error), "PUBLISH: %s", reply->str);
+      log_msg(cfg, error, "PUBLISH: %s", reply->str);
       break;
     }
     freeReplyObject(reply);
@@ -252,7 +248,7 @@ ping(cfg_t *cfg) {
     return false; /* reconnect failure */
 
   if (cfg->conn->err) {
-    snprintf(cfg->error, sizeof(cfg->error), "connection error: %s", cfg->conn->errstr);
+    log_msg(cfg, error, "connection error: %s", cfg->conn->errstr);
     return false;
   }
 
@@ -266,7 +262,7 @@ ping(cfg_t *cfg) {
   if (reply) {
     bool result = true;
     if (reply->type == REDIS_REPLY_ERROR) {
-      strlcpy(cfg->error, reply->str, sizeof(cfg->error));
+      log_msg(cfg, error, "%s", reply->str);
       result = false;
     }
     freeReplyObject(reply);

t/CMakeLists.txt

@@ -8,7 +8,6 @@ add_executable("t_matches"      "t_matches.c" "${SRC_DIR}/strlcpy.c" "${SRC_DIR}
 add_executable("t_ipaddr"       "t_ipaddr.c"  "${SRC_DIR}/strlcpy.c" "${SRC_DIR}/matches.c" "${SRC_DIR}/ipaddr.c")
 add_executable("t_statefile"    "t_statefile.c"  "${SRC_DIR}/strlcpy.c" "${SRC_DIR}/matches.c" "${SRC_DIR}/ipaddr.c" "${SRC_DIR}/statefile.c" "${SRC_DIR}/log.c")
 add_executable("t_config_param" "t_config_param.c" "${SRC_DIR}/strlcpy.c" "${SRC_DIR}/config.c" "${SRC_DIR}/log.c")
-add_executable("t_backend_usage" "t_backend_usage.c" "${SRC_DIR}/strlcpy.c")
 
 add_test("tests/f2b_buf_*"     "t_buf")
 add_test("tests/f2b_cmd_*"     "t_cmd")
@@ -16,7 +15,6 @@ add_test("tests/f2b_matches_*" "t_matches")
 add_test("tests/f2b_ipaddr_*"  "t_ipaddr")
 add_test("tests/f2b_statefile_*"  "t_statefile")
 add_test("tests/f2b_config_param*" "t_config_param")
-add_test("tests/backend/usage_*"   "t_backend_usage")
 
 add_executable("t_filter_preg" "t_filters.c" "${SRC_DIR}/filters/preg.c" "${SRC_DIR}/strlcpy.c")
 add_test("tests/filter/preg"   "t_filter_preg")

t/t_backend_usage.c

@@ -1,24 +0,0 @@
-#include <assert.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-
-#include "../src/strlcpy.h"
-#include "../src/backends/backend.h"
-#include "../src/backends/shared.c"
-
-int main() {
-  assert(usage_inc("test1") == 1);
-  assert(usage_inc("test1") == 2);
-  assert(usage_inc("test2") == 1);
-  assert(usage_inc("test1") == 3);
-
-  assert(usage_dec("test1") == 2);
-  assert(usage_dec("test1") == 1);
-  assert(usage_dec("test1") == 0);
-  assert(usage_dec("test1") == 0);
-
-  assert(usage_dec("test3") == 0);
-
-  return 0;
-}