snmp plugin: codestyle

[collectd.git] / src / snmp.c

diff --git a/src/snmp.c b/src/snmp.c
index dde6079..27ce3c6 100644 (file)
--- a/src/snmp.c
+++ b/src/snmp.c
@@ -116,6 +116,13 @@ struct csnmp_table_values_s {
 };
 typedef struct csnmp_table_values_s csnmp_table_values_t;
 
+typedef enum {
+  OID_TYPE_SKIP = 0,
+  OID_TYPE_VARIABLE,
+  OID_TYPE_INSTANCE,
+  OID_TYPE_HOST,
+} csnmp_oid_type_t;
+
 /*
  * Private variables
  */
@@ -889,8 +896,8 @@ static void csnmp_host_open_session(host_definition_t *host) {
 
 /* TODO: Check if negative values wrap around. Problem: negative temperatures.
  */
-static value_t csnmp_value_list_to_value(struct variable_list *vl, int type,
-                                         double scale, double shift,
+static value_t csnmp_value_list_to_value(const struct variable_list *vl,
+                                         int type, double scale, double shift,
                                          const char *host_name,
                                          const char *data_name) {
   value_t ret;
@@ -1095,31 +1102,24 @@ static int csnmp_strvbcopy(char *dst, /* {{{ */
 
 static int csnmp_instance_list_add(csnmp_list_instances_t **head,
                                    csnmp_list_instances_t **tail,
-                                   const struct snmp_pdu *res,
+                                   const struct variable_list *vb,
                                    const host_definition_t *hd,
                                    const data_definition_t *dd) {
-  csnmp_list_instances_t *il;
-  struct variable_list *vb;
-  oid_t vb_name;
-  int status;
 
-  /* Set vb on the last variable */
-  for (vb = res->variables; (vb != NULL) && (vb->next_variable != NULL);
-       vb = vb->next_variable)
-    /* do nothing */;
   if (vb == NULL)
     return -1;
 
+  oid_t vb_name;
   csnmp_oid_init(&vb_name, vb->name, vb->name_length);
 
-  il = calloc(1, sizeof(*il));
+  csnmp_list_instances_t *il = calloc(1, sizeof(*il));
   if (il == NULL) {
     ERROR("snmp plugin: calloc failed.");
     return -1;
   }
   il->next = NULL;
 
-  status = csnmp_oid_suffix(&il->suffix, &vb_name, &dd->instance.oid);
+  int status = csnmp_oid_suffix(&il->suffix, &vb_name, &dd->instance.oid);
   if (status != 0) {
     sfree(il);
     return status;
@@ -1339,13 +1339,17 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) {
 
   const data_set_t *ds;
 
-  size_t oid_list_len = data->values_len + 1;
+  size_t oid_list_len = data->values_len;
+
+  if (data->instance.oid.oid_len > 0)
+    oid_list_len++;
+
   /* Holds the last OID returned by the device. We use this in the GETNEXT
    * request to proceed. */
   oid_t oid_list[oid_list_len];
   /* Set to false when an OID has left its subtree so we don't re-request it
    * again. */
-  bool oid_list_todo[oid_list_len];
+  csnmp_oid_type_t oid_list_todo[oid_list_len];
 
   int status;
   size_t i;
@@ -1382,15 +1386,16 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) {
   }
   assert(data->values_len > 0);
 
+  for (i = 0; i < data->values_len; i++)
+    oid_list_todo[i] = OID_TYPE_VARIABLE;
+
   /* We need a copy of all the OIDs, because GETNEXT will destroy them. */
   memcpy(oid_list, data->values, data->values_len * sizeof(oid_t));
-  if (data->instance.oid.oid_len > 0)
-    memcpy(oid_list + data->values_len, &data->instance.oid, sizeof(oid_t));
-  else /* no InstanceFrom option specified. */
-    oid_list_len--;
 
-  for (i = 0; i < oid_list_len; i++)
-    oid_list_todo[i] = 1;
+  if (data->instance.oid.oid_len > 0) {
+    memcpy(oid_list + i, &data->instance.oid, sizeof(oid_t));
+    oid_list_todo[i] = OID_TYPE_INSTANCE;
+  }
 
   /* We're going to construct n linked lists, one for each "value".
    * value_list_head will contain pointers to the heads of these linked lists,
@@ -1519,8 +1524,8 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) {
       }
 
       /* An instance is configured and the res variable we process is the
-       * instance value (last index) */
-      if ((data->instance.oid.oid_len > 0) && (i == data->values_len)) {
+       * instance value */
+      if (oid_list_todo[i] == OID_TYPE_INSTANCE) {
         if ((vb->type == SNMP_ENDOFMIBVIEW) ||
             (snmp_oid_ncompare(
                  data->instance.oid.oid, data->instance.oid.oid_len, vb->name,
@@ -1534,14 +1539,19 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) {
         /* Allocate a new `csnmp_list_instances_t', insert the instance name and
          * add it to the list */
         if (csnmp_instance_list_add(&instance_list_head, &instance_list_tail,
-                                    res, host, data) != 0) {
+                                    vb, host, data) != 0) {
           ERROR("snmp plugin: host %s: csnmp_instance_list_add failed.",
                 host->name);
           status = -1;
           break;
         }
+      } else if (oid_list_todo[i] == OID_TYPE_HOST) {
+        /* todo */
+        assert(1 == 0);
       } else /* The variable we are processing is a normal value */
       {
+        assert(oid_list_todo[i] == OID_TYPE_VARIABLE);
+
         csnmp_table_values_t *vt;
         oid_t vb_name;
         oid_t suffix;
@@ -1549,6 +1559,11 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) {
 
         csnmp_oid_init(&vb_name, vb->name, vb->name_length);
 
+        DEBUG(
+            "snmp plugin: src.oid_len = %d root.oid_len = %d is_endofmib = %s",
+            vb_name.oid_len, (data->values + i)->oid_len,
+            (vb->type == SNMP_ENDOFMIBVIEW) ? "true" : "false");
+
         /* Calculate the current suffix. This is later used to check that the
          * suffix is increasing. This also checks if we left the subtree */
         ret = csnmp_oid_suffix(&suffix, &vb_name, data->values + i);
@@ -1561,8 +1576,7 @@ static int csnmp_read_table(host_definition_t *host, data_definition_t *data) {
         }
 
         /* Make sure the OIDs returned by the agent are increasing. Otherwise
-         * our
-         * table matching algorithm will get confused. */
+         * our table matching algorithm will get confused. */
         if ((value_list_tail[i] != NULL) &&
             (csnmp_oid_compare(&suffix, &value_list_tail[i]->suffix) <= 0)) {
           DEBUG("snmp plugin: host = %s; data = %s; i = %" PRIsz "; "