dbi plugin: Implement support for numeric options.

Fixes Github issue #233.

diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod
index c5d7400a8e0c0af50c13e204d397998b16b96bb2..5092bbe66cf10e77391c24218ec5c8193dba55e2 100644 (file)
--- a/src/collectd.conf.pod
+++ b/src/collectd.conf.pod
@@ -1360,11 +1360,23 @@ documentation for each driver, somewhere at
 L<http://libdbi-drivers.sourceforge.net/>. However, the options "host",
 "username", "password", and "dbname" seem to be deE<nbsp>facto standards.
 
+DBDs can register two types of options: String options and numeric options. The
+plugin will use the C<dbi_conn_set_option> function when the configuration
+provides a string and the C<dbi_conn_require_option_numeric> function when the
+configuration provides a number. So these two lines will actually result in
+different calls being used:
+
+  DriverOption "Port" 1234      # numeric
+  DriverOption "Port" "1234"    # string
+
 Unfortunately, drivers are not too keen to report errors when an unknown option
 is passed to them, so invalid settings here may go unnoticed. This is not the
 plugin's fault, it will report errors if it gets them from the libraryE<nbsp>/
 the driver. If a driver complains about an option, the plugin will dump a
-complete list of all options understood by that driver to the log.
+complete list of all options understood by that driver to the log. There is no
+way to programatically find out if an option expects a string or a numeric
+argument, so you will have to refer to the appropriate DBD's documentation to
+find this out. Sorry.
 
 =item B<SelectDB> I<Database>
 

diff --git a/src/dbi.c b/src/dbi.c
index e15de3e89075bf1380b6f7ad57566f5a2948d7f5..0c1982d71719fc2187beeb6d4240da76ad87253d 100644 (file)
--- a/src/dbi.c
+++ b/src/dbi.c
@@ -1,6 +1,6 @@
 /**
  * collectd - src/dbi.c
- * Copyright (C) 2008,2009  Florian octo Forster
+ * Copyright (C) 2008-2013  Florian octo Forster
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU General Public License as published by the
@@ -16,7 +16,7 @@
  * 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
  *
  * Authors:
- *   Florian octo Forster <octo at verplant.org>
+ *   Florian octo Forster <octo at collectd.org>
  **/
 
 #include "collectd.h"
@@ -33,7 +33,12 @@
 struct cdbi_driver_option_s /* {{{ */
 {
   char *key;
-  char *value;
+  union
+  {
+    char *string;
+    int numeric;
+  } value;
+  _Bool is_numeric;
 };
 typedef struct cdbi_driver_option_s cdbi_driver_option_t; /* }}} */
 
@@ -159,7 +164,8 @@ static void cdbi_database_free (cdbi_database_t *db) /* {{{ */
   for (i = 0; i < db->driver_options_num; i++)
   {
     sfree (db->driver_options[i].key);
-    sfree (db->driver_options[i].value);
+    if (!db->driver_options[i].is_numeric)
+      sfree (db->driver_options[i].value.string);
   }
   sfree (db->driver_options);
 
@@ -200,10 +206,11 @@ static int cdbi_config_add_database_driver_option (cdbi_database_t *db, /* {{{ *
 
   if ((ci->values_num != 2)
       || (ci->values[0].type != OCONFIG_TYPE_STRING)
-      || (ci->values[1].type != OCONFIG_TYPE_STRING))
+      || ((ci->values[1].type != OCONFIG_TYPE_STRING)
+        && (ci->values[1].type != OCONFIG_TYPE_NUMBER)))
   {
     WARNING ("dbi plugin: The `DriverOption' config option "
-        "needs exactly two string arguments.");
+        "needs exactly two arguments.");
     return (-1);
   }
 
@@ -217,6 +224,7 @@ static int cdbi_config_add_database_driver_option (cdbi_database_t *db, /* {{{ *
 
   db->driver_options = option;
   option = db->driver_options + db->driver_options_num;
+  memset (option, 0, sizeof (*option));
 
   option->key = strdup (ci->values[0].value.string);
   if (option->key == NULL)
@@ -225,12 +233,21 @@ static int cdbi_config_add_database_driver_option (cdbi_database_t *db, /* {{{ *
     return (-1);
   }
 
-  option->value = strdup (ci->values[1].value.string);
-  if (option->value == NULL)
+  if (ci->values[1].type == OCONFIG_TYPE_STRING)
   {
-    ERROR ("dbi plugin: strdup failed.");
-    sfree (option->key);
-    return (-1);
+    option->value.string = strdup (ci->values[1].value.string);
+    if (option->value.string == NULL)
+    {
+      ERROR ("dbi plugin: strdup failed.");
+      sfree (option->key);
+      return (-1);
+    }
+  }
+  else
+  {
+    assert (ci->values[1].type == OCONFIG_TYPE_NUMBER);
+    option->value.numeric = (int) (ci->values[1].value.number + .5);
+    option->is_numeric = 1;
   }
 
   db->driver_options_num++;
@@ -661,24 +678,38 @@ static int cdbi_connect_database (cdbi_database_t *db) /* {{{ */
    * trouble finding out how to configure the plugin correctly.. */
   for (i = 0; i < db->driver_options_num; i++)
   {
-    DEBUG ("dbi plugin: cdbi_connect_database (%s): "
-        "key = %s; value = %s;",
-        db->name,
-        db->driver_options[i].key,
-        db->driver_options[i].value);
+    if (db->driver_options[i].is_numeric)
+    {
+      status = dbi_conn_set_option_numeric (connection,
+          db->driver_options[i].key, db->driver_options[i].value.numeric);
+      if (status != 0)
+      {
+        char errbuf[1024];
+        ERROR ("dbi plugin: cdbi_connect_database (%s): "
+            "dbi_conn_set_option_numeric (\"%s\", %i) failed: %s.",
+            db->name,
+            db->driver_options[i].key, db->driver_options[i].value.numeric,
+            cdbi_strerror (connection, errbuf, sizeof (errbuf)));
+      }
+    }
+    else
+    {
+      status = dbi_conn_set_option (connection,
+          db->driver_options[i].key, db->driver_options[i].value.string);
+      if (status != 0)
+      {
+        char errbuf[1024];
+        ERROR ("dbi plugin: cdbi_connect_database (%s): "
+            "dbi_conn_set_option (\"%s\", \"%s\") failed: %s.",
+            db->name,
+            db->driver_options[i].key, db->driver_options[i].value.string,
+            cdbi_strerror (connection, errbuf, sizeof (errbuf)));
+      }
+    }
 
-    status = dbi_conn_set_option (connection,
-        db->driver_options[i].key, db->driver_options[i].value);
     if (status != 0)
     {
-      char errbuf[1024];
-      const char *opt;
-
-      ERROR ("dbi plugin: cdbi_connect_database (%s): "
-          "dbi_conn_set_option (%s, %s) failed: %s.",
-          db->name,
-          db->driver_options[i].key, db->driver_options[i].value,
-          cdbi_strerror (connection, errbuf, sizeof (errbuf)));
+      char const *opt;
 
       INFO ("dbi plugin: This is a list of all options understood "
           "by the `%s' driver:", db->driver);