sensors plugin: While I was at it I reorganized the code a little so it's easier...

For instance I moved the name to type conversion out of the main loop and into
a separate function, likewise the chip to string representation conversion. The
`submit' function is now very careful that all strings are null terminated and
that no buffer overflow may occur.

diff --git a/src/sensors.c b/src/sensors.c
index 9319f20..1289d4b 100644 (file)
--- a/src/sensors.c
+++ b/src/sensors.c
@@ -42,17 +42,22 @@
 # define SENSORS_API_VERSION 0x000
 #endif
 
-#define SENSOR_TYPE_VOLTAGE     0
-#define SENSOR_TYPE_FANSPEED    1
-#define SENSOR_TYPE_TEMPERATURE 2
-#define SENSOR_TYPE_UNKNOWN     3
-
+/*
+ * The sensors library prior to version 3.0 (internal version 0x400) didn't
+ * report the type of values, only a name. The following lists are there to
+ * convert from the names to the type. They are not used with the new
+ * interface.
+ */
 #if SENSORS_API_VERSION < 0x400
-static char *sensor_to_type[] =
+static char *sensor_type_name_map[] =
 {
+# define SENSOR_TYPE_VOLTAGE     0
        "voltage",
+# define SENSOR_TYPE_FANSPEED    1
        "fanspeed",
+# define SENSOR_TYPE_TEMPERATURE 2
        "temperature",
+# define SENSOR_TYPE_UNKNOWN     3
        NULL
 };
 
@@ -63,9 +68,7 @@ struct sensors_labeltypes_s
 };
 typedef struct sensors_labeltypes_s sensors_labeltypes_t;
 
-/*
- * finite list of known labels extracted from lm_sensors
- */
+/* finite list of known labels extracted from lm_sensors */
 static sensors_labeltypes_t known_features[] = 
 {
        { "fan1", SENSOR_TYPE_FANSPEED },
@@ -128,10 +131,9 @@ static int known_features_num = STATIC_ARRAY_SIZE (known_features);
 static const char *config_keys[] =
 {
        "Sensor",
-       "IgnoreSelected",
-       NULL
+       "IgnoreSelected"
 };
-static int config_keys_num = 2;
+static int config_keys_num = STATIC_ARRAY_SIZE (config_keys);
 
 #if SENSORS_API_VERSION < 0x400
 typedef struct featurelist
@@ -171,6 +173,52 @@ featurelist_t *first_feature = NULL;
 static ignorelist_t *sensor_list;
 static time_t sensors_config_mtime = 0;
 
+#if SENSORS_API_VERSION < 0x400
+/* full chip name logic borrowed from lm_sensors */
+static int sensors_snprintf_chip_name (char *buf, size_t buf_size,
+               const sensors_chip_name *chip)
+{
+       int status = -1;
+
+       if (chip->bus == SENSORS_CHIP_NAME_BUS_ISA)
+       {
+               status = snprintf (buf, buf_size,
+                               "%s-isa-%04x",
+                               chip->prefix,
+                               chip->addr);
+       }
+       else if (chip->bus == SENSORS_CHIP_NAME_BUS_DUMMY)
+       {
+               snprintf (buf, buf_size, "%s-%s-%04x",
+                               chip->prefix,
+                               chip->busname,
+                               chip->addr);
+       }
+       else
+       {
+               snprintf (buf, buf_size, "%s-i2c-%d-%02x",
+                               chip->prefix,
+                               chip->bus,
+                               chip->addr);
+       }
+
+       return (status);
+} /* int sensors_snprintf_chip_name */
+
+static int sensors_feature_name_to_type (const char *name)
+{
+       int i;
+
+       /* Yes, this is slow, but it's only ever done during initialization, so
+        * it's a one time cost.. */
+       for (i = 0; i < known_features_num; i++)
+               if (strcasecmp (known_features[i].label, name) == 0)
+                       return (known_features[i].type);
+
+       return (SENSOR_TYPE_UNKNOWN);
+} /* int sensors_feature_name_to_type */
+#endif
+
 static int sensors_config (const char *key, const char *value)
 {
        if (sensor_list == NULL)
@@ -281,7 +329,8 @@ static int sensors_load_conf (void)
                while (42)
                {
                        const sensors_feature_data *feature;
-                       int i;
+                       int feature_type;
+                       featurelist_t *fl;
 
                        feature = sensors_get_all_features (*chip,
                                        &feature_num0, &feature_num1);
@@ -294,63 +343,43 @@ static int sensors_load_conf (void)
                        if (feature->mapping != SENSORS_NO_MAPPING)
                                continue;
 
-                       /* Only known features */
-                       for (i = 0; i < known_features_num; i++)
-                       {
-                               featurelist_t *fl;
-
-                               if (strcasecmp (feature->name,
-                                                       known_features[i].label)
-                                               != 0)
-                                       continue;
-
-                               /* skip ignored in sensors.conf */
-                               if (sensors_get_ignored (*chip, feature->number) == 0)
-                                       break;
-
-                               DEBUG ("Adding feature: %s-%s-%s",
-                                               chip->prefix,
-                                               sensor_to_type[known_features[i].type],
-                                               feature->name);
-
-                               fl = (featurelist_t *) malloc (sizeof (featurelist_t));
-                               if (fl == NULL)
-                               {
-                                       ERROR ("sensors plugin: malloc failed.");
-                                       continue;
-                               }
-                               memset (fl, '\0', sizeof (featurelist_t));
-
-                               fl->chip = chip;
-                               fl->data = feature;
-                               fl->type = known_features[i].type;
+                       /* skip ignored in sensors.conf */
+                       if (sensors_get_ignored (*chip, feature->number) == 0)
+                               break;
 
-                               if (first_feature == NULL)
-                                       first_feature = fl;
-                               else
-                                       last_feature->next = fl;
-                               last_feature = fl;
+                       feature_type = sensors_feature_name_to_type (
+                                       feature->name);
+                       if (feature_type == SENSOR_TYPE_UNKNOWN)
+                               continue;
 
-                               /* stop searching known features at first found */
-                               break;
-                       } /* for i */
+                       fl = (featurelist_t *) malloc (sizeof (featurelist_t));
+                       if (fl == NULL)
+                       {
+                               ERROR ("sensors plugin: malloc failed.");
+                               continue;
+                       }
+                       memset (fl, '\0', sizeof (featurelist_t));
+
+                       fl->chip = chip;
+                       fl->data = feature;
+                       fl->type = feature_type;
+
+                       if (first_feature == NULL)
+                               first_feature = fl;
+                       else
+                               last_feature->next = fl;
+                       last_feature = fl;
                } /* while sensors_get_all_features */
        } /* while sensors_get_detected_chips */
 /* #endif SENSORS_API_VERSION < 0x400 */
 
 #elif (SENSORS_API_VERSION >= 0x400) && (SENSORS_API_VERSION < 0x500)
        chip_num = 0;
-       DEBUG ("sensors plugin: Calling sensors_get_detected_chips (NULL, %p)..", (void *) &chip_num);
        while ((chip = sensors_get_detected_chips (NULL, &chip_num)) != NULL)
        {
                const sensors_feature *feature;
                int feature_num = 0;
 
-               DEBUG ("sensors plugin: Handling chip with prefix `%s'",
-                               (chip->prefix == NULL)
-                               ? "(nil)"
-                               : chip->prefix);
-
                while ((feature = sensors_get_features (chip, &feature_num)) != NULL)
                {
                        const sensors_subfeature *subfeature;
@@ -425,7 +454,7 @@ static void sensors_submit (const char *plugin_instance,
 
        status = snprintf (match_key, sizeof (match_key), "%s/%s-%s",
                        plugin_instance, type, type_instance);
-       if ((status < 1) || (status >= sizeof (match_key)))
+       if (status < 1)
                return;
        match_key[sizeof (match_key) - 1] = '\0';
 
@@ -441,10 +470,16 @@ static void sensors_submit (const char *plugin_instance,
        vl.values = values;
        vl.values_len = 1;
        vl.time = time (NULL);
-       strcpy (vl.host, hostname_g);
-       strcpy (vl.plugin, "sensors");
-       strcpy (vl.plugin_instance, plugin_instance);
-       strcpy (vl.type_instance, type_instance);
+
+       strncpy (vl.host, hostname_g, sizeof (vl.host));
+       vl.host[sizeof (vl.host) - 1] = '\0';
+       strncpy (vl.plugin, "sensors", sizeof (vl.plugin));
+       vl.plugin[sizeof (vl.plugin) - 1] = '\0';
+       strncpy (vl.plugin_instance, plugin_instance,
+                       sizeof (vl.plugin_instance));
+       vl.plugin_instance[sizeof (vl.plugin_instance) - 1] = '\0';
+       strncpy (vl.type_instance, type_instance, sizeof (vl.type_instance));
+       vl.type_instance[sizeof (vl.type_instance) - 1] = '\0';
 
        plugin_dispatch_values (type, &vl);
 } /* void sensors_submit */
@@ -469,35 +504,18 @@ static int sensors_read (void)
                if (status < 0)
                        continue;
 
-               /* full chip name logic borrowed from lm_sensors */
-               if (fl->chip->bus == SENSORS_CHIP_NAME_BUS_ISA)
-               {
-                       snprintf (plugin_instance, DATA_MAX_NAME_LEN,
-                                       "%s-isa-%04x",
-                                       fl->chip->prefix,
-                                       fl->chip->addr);
-               }
-               else if (fl->chip->bus == SENSORS_CHIP_NAME_BUS_DUMMY)
-               {
-                       snprintf (plugin_instance, 512, "%s-%s-%04x",
-                                       fl->chip->prefix,
-                                       fl->chip->busname,
-                                       fl->chip->addr);
-               }
-               else
-               {
-                       snprintf (plugin_instance, 512, "%s-i2c-%d-%02x",
-                                       fl->chip->prefix,
-                                       fl->chip->bus,
-                                       fl->chip->addr);
-               }
+               status = sensors_snprintf_chip_name (plugin_instance,
+                               sizeof (plugin_instance), fl->chip);
+               if (status < 0)
+                       continue;
                plugin_instance[sizeof (plugin_instance) - 1] = '\0';
 
-               strncpy (type_instance, fl->data->name, sizeof (type_instance));
+               strncpy (type_instance, fl->data->name,
+                               sizeof (type_instance));
                type_instance[sizeof (type_instance) - 1] = '\0';
 
                sensors_submit (plugin_instance,
-                               sensor_to_type[fl->type],
+                               sensor_type_name_map[fl->type],
                                type_instance,
                                value);
        } /* for fl = first_feature .. NULL */