Merge branch 'collectd-5.8'
[collectd.git] / src / snmp_agent.c
index c732bb6..48d9f86 100644 (file)
@@ -1,17 +1,17 @@
 /**
  * collectd - src/snmp_agent.c
  *
- * Copyright(c) 2016 Intel Corporation. All rights reserved.
+ * Copyright(c) 2017 Intel Corporation. All rights reserved.
  *
- * Permission is hereby granted, free of charge, to any person obtaining a copy of
- * this software and associated documentation files (the "Software"), to deal in
- * the Software without restriction, including without limitation the rights to
- * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies
- * of the Software, and to permit persons to whom the Software is furnished to do
- * so, subject to the following conditions:
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
  *
- * The above copyright notice and this permission notice shall be included in all
- * copies or substantial portions of the Software.
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
  *
  * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
  * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
@@ -34,7 +34,9 @@
 #include "utils_llist.h"
 
 #include <net-snmp/net-snmp-config.h>
+
 #include <net-snmp/net-snmp-includes.h>
+
 #include <net-snmp/agent/net-snmp-agent-includes.h>
 
 #define PLUGIN_NAME "snmp_agent"
@@ -69,7 +71,7 @@ struct data_definition_s {
   char *type;
   char *type_instance;
   const table_definition_t *table;
-  _Bool is_instance;
+  bool is_instance;
   oid_t *oids;
   size_t oids_len;
   double scale;
@@ -88,7 +90,7 @@ struct snmp_agent_ctx_s {
 };
 typedef struct snmp_agent_ctx_s snmp_agent_ctx_t;
 
-snmp_agent_ctx_t *g_agent = NULL;
+static snmp_agent_ctx_t *g_agent;
 
 #define CHECK_DD_TYPE(_dd, _p, _pi, _t, _ti)                                   \
   (_dd->plugin ? !strcmp(_dd->plugin, _p) : 0) &&                              \
@@ -96,6 +98,7 @@ snmp_agent_ctx_t *g_agent = NULL;
       (_dd->type ? !strcmp(_dd->type, _t) : 0) &&                              \
       (_dd->type_instance ? !strcmp(_dd->type_instance, _ti) : 1)
 
+static int snmp_agent_shutdown(void);
 static void *snmp_agent_thread_run(void *arg);
 static int snmp_agent_register_oid(oid_t *oid, Netsnmp_Node_Handler *handler);
 static int snmp_agent_set_vardata(void *dst_buf, size_t *dst_buf_len,
@@ -106,13 +109,13 @@ static int snmp_agent_unregister_oid_index(oid_t *oid, int index);
 static u_char snmp_agent_get_asn_type(oid *oid, size_t oid_len) {
   struct tree *node = get_tree(oid, oid_len, g_agent->tp);
 
-  return (node != NULL ? mib_to_asn_type(node->type) : 0);
+  return (node != NULL) ? mib_to_asn_type(node->type) : 0;
 }
 
 static char *snmp_agent_get_oid_name(oid *oid, size_t oid_len) {
   struct tree *node = get_tree(oid, oid_len, g_agent->tp);
 
-  return (node != NULL ? node->label : NULL);
+  return (node != NULL) ? node->label : NULL;
 }
 
 static int snmp_agent_oid_to_string(char *buf, size_t buf_size,
@@ -121,11 +124,11 @@ static int snmp_agent_oid_to_string(char *buf, size_t buf_size,
   char *oid_str_ptr[MAX_OID_LEN];
 
   for (size_t i = 0; i < o->oid_len; i++) {
-    ssnprintf(oid_str[i], sizeof(oid_str[i]), "%lu", (unsigned long)o->oid[i]);
+    snprintf(oid_str[i], sizeof(oid_str[i]), "%lu", (unsigned long)o->oid[i]);
     oid_str_ptr[i] = oid_str[i];
   }
 
-  return (strjoin(buf, buf_size, oid_str_ptr, o->oid_len, "."));
+  return strjoin(buf, buf_size, oid_str_ptr, o->oid_len, ".");
 }
 
 static void snmp_agent_dump_data(void) {
@@ -161,9 +164,9 @@ static void snmp_agent_dump_data(void) {
         DEBUG(PLUGIN_NAME ":     Type: %s", dd->type);
       if (dd->type_instance)
         DEBUG(PLUGIN_NAME ":     TypeInstance: %s", dd->type_instance);
-      for (int i = 0; i < dd->oids_len; i++) {
+      for (size_t i = 0; i < dd->oids_len; i++) {
         snmp_agent_oid_to_string(oid_str, sizeof(oid_str), &dd->oids[i]);
-        DEBUG(PLUGIN_NAME ":     OID[%d]: %s", i, oid_str);
+        DEBUG(PLUGIN_NAME ":     OID[%" PRIsz "]: %s", i, oid_str);
       }
       DEBUG(PLUGIN_NAME ":   Scale: %g", dd->scale);
       DEBUG(PLUGIN_NAME ":   Shift: %g", dd->shift);
@@ -185,9 +188,9 @@ static void snmp_agent_dump_data(void) {
       DEBUG(PLUGIN_NAME ":   Type: %s", dd->type);
     if (dd->type_instance)
       DEBUG(PLUGIN_NAME ":   TypeInstance: %s", dd->type_instance);
-    for (int i = 0; i < dd->oids_len; i++) {
+    for (size_t i = 0; i < dd->oids_len; i++) {
       snmp_agent_oid_to_string(oid_str, sizeof(oid_str), &dd->oids[i]);
-      DEBUG(PLUGIN_NAME ":   OID[%d]: %s", i, oid_str);
+      DEBUG(PLUGIN_NAME ":   OID[%" PRIsz "]: %s", i, oid_str);
     }
     DEBUG(PLUGIN_NAME ":   Scale: %g", dd->scale);
     DEBUG(PLUGIN_NAME ":   Shift: %g", dd->shift);
@@ -208,20 +211,20 @@ static int snmp_agent_validate_data(void) {
       if (!dd->plugin) {
         ERROR(PLUGIN_NAME ": Plugin not defined for '%s'.'%s'", td->name,
               dd->name);
-        return (-EINVAL);
+        return -EINVAL;
       }
 
       if (dd->plugin_instance) {
         ERROR(PLUGIN_NAME ": PluginInstance should not be defined for table "
                           "data type '%s'.'%s'",
               td->name, dd->name);
-        return (-EINVAL);
+        return -EINVAL;
       }
 
       if (dd->oids_len == 0) {
         ERROR(PLUGIN_NAME ": No OIDs defined for '%s'.'%s'", td->name,
               dd->name);
-        return (-EINVAL);
+        return -EINVAL;
       }
 
       if (dd->is_instance) {
@@ -230,7 +233,7 @@ static int snmp_agent_validate_data(void) {
           ERROR(PLUGIN_NAME ": Type and TypeInstance are not valid for "
                             "instance data '%s'.'%s'",
                 td->name, dd->name);
-          return (-EINVAL);
+          return -EINVAL;
         }
 
         if (dd->oids_len > 1) {
@@ -238,14 +241,14 @@ static int snmp_agent_validate_data(void) {
               PLUGIN_NAME
               ": Only one OID should be specified for instance data '%s'.'%s'",
               td->name, dd->name);
-          return (-EINVAL);
+          return -EINVAL;
         }
       } else {
 
         if (!dd->type) {
           ERROR(PLUGIN_NAME ": Type not defined for data '%s'.'%s'", td->name,
                 dd->name);
-          return (-EINVAL);
+          return -EINVAL;
         }
       }
     }
@@ -256,79 +259,139 @@ static int snmp_agent_validate_data(void) {
 
     if (!dd->plugin) {
       ERROR(PLUGIN_NAME ": Plugin not defined for '%s'", dd->name);
-      return (-EINVAL);
+      return -EINVAL;
     }
 
     if (dd->oids_len == 0) {
       ERROR(PLUGIN_NAME ": No OIDs defined for '%s'", dd->name);
-      return (-EINVAL);
+      return -EINVAL;
     }
 
     if (dd->is_instance) {
       ERROR(PLUGIN_NAME
             ": Instance flag can't be specified for scalar data '%s'",
             dd->name);
-      return (-EINVAL);
+      return -EINVAL;
     }
 
     if (!dd->type) {
       ERROR(PLUGIN_NAME ": Type not defined for data '%s'", dd->name);
-      return (-EINVAL);
+      return -EINVAL;
+    }
+  }
+
+  return 0;
+}
+
+static void snmp_agent_generate_oid2string(oid_t *oid, size_t offset,
+                                           char *key) {
+  int key_len = oid->oid[offset];
+  int i;
+
+  for (i = 0; i < key_len && offset < oid->oid_len; i++)
+    key[i] = oid->oid[++offset];
+
+  key[i] = '\0';
+}
+
+static int snmp_agent_generate_string2oid(oid_t *oid, const char *key) {
+  int key_len = strlen(key);
+
+  oid->oid[oid->oid_len++] = key_len;
+  for (int i = 0; i < key_len; i++) {
+    oid->oid[oid->oid_len++] = key[i];
+    if (oid->oid_len >= MAX_OID_LEN) {
+      ERROR(PLUGIN_NAME ": Conversion key string %s to OID failed", key);
+      return -EINVAL;
     }
   }
 
-  return (0);
+  return 0;
+}
+
+static int snmp_agent_register_oid_string(oid_t *oid, const char *key,
+                                          Netsnmp_Node_Handler *handler) {
+  oid_t new_oid;
+
+  memcpy(&new_oid, oid, sizeof(*oid));
+  int ret = snmp_agent_generate_string2oid(&new_oid, key);
+  if (ret != 0)
+    return ret;
+
+  return snmp_agent_register_oid(&new_oid, handler);
+}
+
+static int snmp_agent_unregister_oid_string(oid_t *oid, const char *key) {
+  oid_t new_oid;
+
+  memcpy(&new_oid, oid, sizeof(*oid));
+  int ret = snmp_agent_generate_string2oid(&new_oid, key);
+  if (ret != 0)
+    return ret;
+
+  return unregister_mib(new_oid.oid, new_oid.oid_len);
 }
 
 static int snmp_agent_table_row_remove(table_definition_t *td,
                                        const char *instance) {
-  int *index;
-  char *ins;
+  int *index = NULL;
+  char *ins = NULL;
 
-  if ((c_avl_get(td->instance_index, instance, (void **)&index) != 0) ||
-      (c_avl_get(td->index_instance, index, (void **)&ins) != 0))
-    return (0);
+  if (td->index_oid.oid_len) {
+    if ((c_avl_get(td->instance_index, instance, (void **)&index) != 0) ||
+        (c_avl_get(td->index_instance, index, (void **)&ins) != 0))
+      return 0;
+  } else {
+    if (c_avl_get(td->instance_index, instance, (void **)&ins) != 0)
+      return 0;
+  }
 
   pthread_mutex_lock(&g_agent->agentx_lock);
 
+  if (td->index_oid.oid_len)
+    snmp_agent_unregister_oid_index(&td->index_oid, *index);
+
   for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) {
     data_definition_t *dd = de->value;
 
-    for (int i = 0; i < dd->oids_len; i++)
-      snmp_agent_unregister_oid_index(&dd->oids[i], *index);
+    for (size_t i = 0; i < dd->oids_len; i++)
+      if (td->index_oid.oid_len)
+        snmp_agent_unregister_oid_index(&dd->oids[i], *index);
+      else
+        snmp_agent_unregister_oid_string(&dd->oids[i], ins);
   }
 
-  snmp_agent_unregister_oid_index(&td->index_oid, *index);
-
   pthread_mutex_unlock(&g_agent->agentx_lock);
 
-  DEBUG(PLUGIN_NAME ": Removed row for '%s' table [%d, %s]", td->name, *index,
-        ins);
+  DEBUG(PLUGIN_NAME ": Removed row for '%s' table [%d, %s]", td->name,
+        (index != NULL) ? *index : -1, ins);
 
   notification_t n = {
-    .severity = NOTIF_WARNING,
-    .time = cdtime(),
-    .plugin = PLUGIN_NAME
-  };
+      .severity = NOTIF_WARNING, .time = cdtime(), .plugin = PLUGIN_NAME};
   sstrncpy(n.host, hostname_g, sizeof(n.host));
   sstrncpy(n.plugin_instance, ins, sizeof(n.plugin_instance));
-  ssnprintf(n.message, sizeof(n.message),
-            "Removed data row from table %s instance %s index %d", td->name,
-            ins, *index);
+  snprintf(n.message, sizeof(n.message),
+           "Removed data row from table %s instance %s index %d", td->name, ins,
+           (index != NULL) ? *index : -1);
   plugin_dispatch_notification(&n);
 
-  c_avl_remove(td->index_instance, index, NULL, (void **)&ins);
-  c_avl_remove(td->instance_index, instance, NULL, (void **)&index);
-  sfree(index);
-  sfree(ins);
+  if (td->index_oid.oid_len) {
+    c_avl_remove(td->index_instance, index, NULL, (void **)&ins);
+    c_avl_remove(td->instance_index, instance, NULL, (void **)&index);
+    sfree(index);
+    sfree(ins);
+  } else {
+    c_avl_remove(td->instance_index, instance, NULL, (void **)&ins);
+    sfree(ins);
+  }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_clear_missing(const value_list_t *vl,
                                     __attribute__((unused)) user_data_t *ud) {
   if (vl == NULL)
-    return (-EINVAL);
+    return -EINVAL;
 
   for (llentry_t *te = llist_head(g_agent->tables); te != NULL; te = te->next) {
     table_definition_t *td = te->value;
@@ -340,13 +403,13 @@ static int snmp_agent_clear_missing(const value_list_t *vl,
         if (CHECK_DD_TYPE(dd, vl->plugin, vl->plugin_instance, vl->type,
                           vl->type_instance)) {
           snmp_agent_table_row_remove(td, vl->plugin_instance);
-          return (0);
+          return 0;
         }
       }
     }
   }
 
-  return (0);
+  return 0;
 }
 
 static void snmp_agent_free_data(data_definition_t **dd) {
@@ -356,19 +419,8 @@ static void snmp_agent_free_data(data_definition_t **dd) {
 
   /* unregister scalar type OID */
   if ((*dd)->table == NULL) {
-    for (int i = 0; i < (*dd)->oids_len; i++)
+    for (size_t i = 0; i < (*dd)->oids_len; i++)
       unregister_mib((*dd)->oids[i].oid, (*dd)->oids[i].oid_len);
-  } else {
-    /* unregister all table OIDs */
-    int *index;
-    char *value;
-
-    c_avl_iterator_t *iter = c_avl_get_iterator((*dd)->table->index_instance);
-    while (c_avl_iterator_next(iter, (void *)&index, (void *)&value) == 0) {
-      for (int i = 0; i < (*dd)->oids_len; i++)
-        snmp_agent_unregister_oid_index(&(*dd)->oids[i], *index);
-    }
-    c_avl_iterator_destroy(iter);
   }
 
   sfree((*dd)->name);
@@ -383,6 +435,43 @@ static void snmp_agent_free_data(data_definition_t **dd) {
   return;
 }
 
+static void snmp_agent_free_table_columns(table_definition_t *td) {
+  if (td->columns == NULL)
+    return;
+
+  for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) {
+    data_definition_t *dd = de->value;
+
+    if (td->index_oid.oid_len) {
+      int *index;
+      char *instance;
+
+      c_avl_iterator_t *iter = c_avl_get_iterator(td->index_instance);
+      while (c_avl_iterator_next(iter, (void *)&index, (void *)&instance) ==
+             0) {
+        for (size_t i = 0; i < dd->oids_len; i++)
+          snmp_agent_unregister_oid_index(&dd->oids[i], *index);
+      }
+      c_avl_iterator_destroy(iter);
+    } else {
+      char *instance;
+
+      c_avl_iterator_t *iter = c_avl_get_iterator(dd->table->instance_index);
+      while (c_avl_iterator_next(iter, (void *)&instance, (void *)&instance) ==
+             0) {
+        for (size_t i = 0; i < dd->oids_len; i++)
+          snmp_agent_unregister_oid_string(&dd->oids[i], instance);
+      }
+      c_avl_iterator_destroy(iter);
+    }
+
+    snmp_agent_free_data(&dd);
+  }
+
+  llist_destroy(td->columns);
+  td->columns = NULL;
+} /* void snmp_agent_free_table_columns */
+
 static void snmp_agent_free_table(table_definition_t **td) {
 
   if (td == NULL || *td == NULL)
@@ -391,23 +480,20 @@ static void snmp_agent_free_table(table_definition_t **td) {
   if ((*td)->size_oid.oid_len)
     unregister_mib((*td)->size_oid.oid, (*td)->size_oid.oid_len);
 
+  /* Unregister Index OIDs */
   if ((*td)->index_oid.oid_len) {
     int *index;
-    char *value;
+    char *instance;
 
     c_avl_iterator_t *iter = c_avl_get_iterator((*td)->index_instance);
-    while (c_avl_iterator_next(iter, (void *)&index, (void *)&value) == 0)
+    while (c_avl_iterator_next(iter, (void *)&index, (void *)&instance) == 0)
       snmp_agent_unregister_oid_index(&(*td)->index_oid, *index);
 
     c_avl_iterator_destroy(iter);
   }
 
-  for (llentry_t *de = llist_head((*td)->columns); de != NULL; de = de->next) {
-    data_definition_t *dd = de->value;
-    snmp_agent_free_data(&dd);
-  }
-
-  llist_destroy((*td)->columns);
+  /* Unregister all table columns and their registered OIDs */
+  snmp_agent_free_table_columns(*td);
 
   void *key = NULL;
   void *value = NULL;
@@ -416,12 +502,15 @@ static void snmp_agent_free_table(table_definition_t **td) {
   c_avl_destroy((*td)->index_instance);
   (*td)->index_instance = NULL;
 
-  while (c_avl_pick((*td)->instance_index, &key, &value) == 0) {
-    sfree(key);
-    sfree(value);
+  if ((*td)->instance_index != NULL) {
+    while (c_avl_pick((*td)->instance_index, &key, &value) == 0) {
+      if (key != value)
+        sfree(key);
+      sfree(value);
+    }
+    c_avl_destroy((*td)->instance_index);
+    (*td)->instance_index = NULL;
   }
-  c_avl_destroy((*td)->instance_index);
-  (*td)->instance_index = NULL;
 
   sfree((*td)->name);
   sfree(*td);
@@ -432,29 +521,29 @@ static void snmp_agent_free_table(table_definition_t **td) {
 static int snmp_agent_form_reply(struct netsnmp_request_info_s *requests,
                                  data_definition_t *dd, char *instance,
                                  int oid_index) {
-  char buf[DATA_MAX_NAME_LEN];
-  format_name(buf, sizeof(buf), hostname_g, dd->plugin,
+  char name[DATA_MAX_NAME_LEN];
+  format_name(name, sizeof(name), hostname_g, dd->plugin,
               instance ? instance : dd->plugin_instance, dd->type,
               dd->type_instance);
-  DEBUG(PLUGIN_NAME ": Identifier '%s'", buf);
+  DEBUG(PLUGIN_NAME ": Identifier '%s'", name);
 
   value_t *values;
   size_t values_num;
   const data_set_t *ds = plugin_get_ds(dd->type);
   if (ds == NULL) {
-    DEBUG(PLUGIN_NAME ": Data set not found for '%s' type", dd->type);
+    ERROR(PLUGIN_NAME ": Data set not found for '%s' type", dd->type);
     return SNMP_NOSUCHINSTANCE;
   }
 
-  int ret = uc_get_value_by_name(buf, &values, &values_num);
+  int ret = uc_get_value_by_name(name, &values, &values_num);
 
   if (ret != 0) {
-    ERROR(PLUGIN_NAME ": Failed to get value for '%s'", buf);
+    ERROR(PLUGIN_NAME ": Failed to get value for '%s'", name);
     return SNMP_NOSUCHINSTANCE;
   }
 
   assert(ds->ds_num == values_num);
-  assert(oid_index < values_num);
+  assert(oid_index < (int)values_num);
 
   char data[DATA_MAX_NAME_LEN];
   size_t data_len = sizeof(data);
@@ -465,13 +554,13 @@ static int snmp_agent_form_reply(struct netsnmp_request_info_s *requests,
   sfree(values);
 
   if (ret != 0) {
-    ERROR(PLUGIN_NAME ": Failed to convert '%s' value to snmp data", buf);
+    ERROR(PLUGIN_NAME ": Failed to convert '%s' value to snmp data", name);
     return SNMP_NOSUCHINSTANCE;
   }
 
   requests->requestvb->type = dd->oids[oid_index].type;
-  snmp_set_var_typed_value(requests->requestvb, requests->requestvb->type, data,
-                           data_len);
+  snmp_set_var_typed_value(requests->requestvb, requests->requestvb->type,
+                           (const u_char *)data, data_len);
 
   return SNMP_ERR_NOERROR;
 }
@@ -506,33 +595,44 @@ snmp_agent_table_oid_handler(struct netsnmp_mib_handler_s *handler,
     for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) {
       data_definition_t *dd = de->value;
 
-      for (int i = 0; i < dd->oids_len; i++) {
+      for (size_t i = 0; i < dd->oids_len; i++) {
         int ret = snmp_oid_ncompare(oid.oid, oid.oid_len, dd->oids[i].oid,
                                     dd->oids[i].oid_len,
                                     MIN(oid.oid_len, dd->oids[i].oid_len));
         if (ret != 0)
           continue;
 
-        if (!td->index_oid.oid_len) {
-          DEBUG(PLUGIN_NAME ": %s:%d NOT IMPLEMENTED", __FUNCTION__, __LINE__);
-          continue;
-        }
-
-        int index = oid.oid[oid.oid_len - 1];
         char *instance;
 
-        ret = c_avl_get(td->index_instance, &index, (void **)&instance);
-        if (ret != 0) {
-          DEBUG(PLUGIN_NAME ": Nonexisting index '%d' requested", index);
-          pthread_mutex_unlock(&g_agent->lock);
-          return SNMP_NOSUCHINSTANCE;
+        if (!td->index_oid.oid_len) {
+          char key[MAX_OID_LEN];
+
+          memset(key, 0, sizeof(key));
+          snmp_agent_generate_oid2string(
+              &oid, MIN(oid.oid_len, dd->oids[i].oid_len), key);
+
+          ret = c_avl_get(td->instance_index, key, (void **)&instance);
+          if (ret != 0) {
+            DEBUG(PLUGIN_NAME ": Nonexisting index string '%s' requested", key);
+            pthread_mutex_unlock(&g_agent->lock);
+            return SNMP_NOSUCHINSTANCE;
+          }
+        } else {
+          int index = oid.oid[oid.oid_len - 1];
+
+          ret = c_avl_get(td->index_instance, &index, (void **)&instance);
+          if (ret != 0) {
+            DEBUG(PLUGIN_NAME ": Nonexisting index '%d' requested", index);
+            pthread_mutex_unlock(&g_agent->lock);
+            return SNMP_NOSUCHINSTANCE;
+          }
         }
 
         if (dd->is_instance) {
           requests->requestvb->type = ASN_OCTET_STR;
-          snmp_set_var_typed_value(requests->requestvb,
-                                   requests->requestvb->type, instance,
-                                   strlen((instance)));
+          snmp_set_var_typed_value(
+              requests->requestvb, requests->requestvb->type,
+              (const u_char *)instance, strlen((instance)));
 
           pthread_mutex_unlock(&g_agent->lock);
 
@@ -592,7 +692,7 @@ static int snmp_agent_table_index_oid_handler(
 
       requests->requestvb->type = ASN_INTEGER;
       snmp_set_var_typed_value(requests->requestvb, requests->requestvb->type,
-                               &index, sizeof(index));
+                               (const u_char *)&index, sizeof(index));
 
       pthread_mutex_unlock(&g_agent->lock);
 
@@ -638,7 +738,7 @@ static int snmp_agent_table_size_oid_handler(
 
       requests->requestvb->type = td->size_oid.type;
       snmp_set_var_typed_value(requests->requestvb, requests->requestvb->type,
-                               &size, sizeof(size));
+                               (const u_char *)&size, sizeof(size));
 
       pthread_mutex_unlock(&g_agent->lock);
 
@@ -679,7 +779,7 @@ snmp_agent_scalar_oid_handler(struct netsnmp_mib_handler_s *handler,
        de = de->next) {
     data_definition_t *dd = de->value;
 
-    for (int i = 0; i < dd->oids_len; i++) {
+    for (size_t i = 0; i < dd->oids_len; i++) {
 
       int ret = snmp_oid_compare(oid.oid, oid.oid_len, dd->oids[i].oid,
                                  dd->oids[i].oid_len);
@@ -717,14 +817,14 @@ static int snmp_agent_register_table_oids(void) {
     for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) {
       data_definition_t *dd = de->value;
 
-      for (int i = 0; i < dd->oids_len; i++) {
+      for (size_t i = 0; i < dd->oids_len; i++) {
         dd->oids[i].type =
             snmp_agent_get_asn_type(dd->oids[i].oid, dd->oids[i].oid_len);
       }
     }
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_register_scalar_oids(void) {
@@ -732,7 +832,7 @@ static int snmp_agent_register_scalar_oids(void) {
   for (llentry_t *e = llist_head(g_agent->scalars); e != NULL; e = e->next) {
     data_definition_t *dd = e->value;
 
-    for (int i = 0; i < dd->oids_len; i++) {
+    for (size_t i = 0; i < dd->oids_len; i++) {
 
       dd->oids[i].type =
           snmp_agent_get_asn_type(dd->oids[i].oid, dd->oids[i].oid_len);
@@ -744,20 +844,20 @@ static int snmp_agent_register_scalar_oids(void) {
     }
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_config_data_oids(data_definition_t *dd,
                                        oconfig_item_t *ci) {
   if (ci->values_num < 1) {
     WARNING(PLUGIN_NAME ": `OIDs' needs at least one argument");
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   for (int i = 0; i < ci->values_num; i++)
     if (ci->values[i].type != OCONFIG_TYPE_STRING) {
       WARNING(PLUGIN_NAME ": `OIDs' needs only string argument");
-      return (-EINVAL);
+      return -EINVAL;
     }
 
   if (dd->oids != NULL)
@@ -765,7 +865,7 @@ static int snmp_agent_config_data_oids(data_definition_t *dd,
   dd->oids_len = 0;
   dd->oids = calloc(ci->values_num, sizeof(*dd->oids));
   if (dd->oids == NULL)
-    return (-ENOMEM);
+    return -ENOMEM;
   dd->oids_len = (size_t)ci->values_num;
 
   for (int i = 0; i < ci->values_num; i++) {
@@ -777,23 +877,23 @@ static int snmp_agent_config_data_oids(data_definition_t *dd,
             ci->values[i].value.string);
       sfree(dd->oids);
       dd->oids_len = 0;
-      return (-1);
+      return -1;
     }
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_config_table_size_oid(table_definition_t *td,
                                             oconfig_item_t *ci) {
   if (ci->values_num < 1) {
     WARNING(PLUGIN_NAME ": `TableSizeOID' is empty");
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   if (ci->values[0].type != OCONFIG_TYPE_STRING) {
     WARNING(PLUGIN_NAME ": `TableSizeOID' needs only string argument");
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   td->size_oid.oid_len = MAX_OID_LEN;
@@ -803,10 +903,10 @@ static int snmp_agent_config_table_size_oid(table_definition_t *td,
     ERROR(PLUGIN_NAME ": Failed to parse table size OID (%s)",
           ci->values[0].value.string);
     td->size_oid.oid_len = 0;
-    return (-EINVAL);
+    return -EINVAL;
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_config_table_index_oid(table_definition_t *td,
@@ -814,12 +914,12 @@ static int snmp_agent_config_table_index_oid(table_definition_t *td,
 
   if (ci->values_num < 1) {
     WARNING(PLUGIN_NAME ": `IndexOID' is empty");
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   if (ci->values[0].type != OCONFIG_TYPE_STRING) {
     WARNING(PLUGIN_NAME ": `IndexOID' needs only string argument");
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   td->index_oid.oid_len = MAX_OID_LEN;
@@ -829,10 +929,10 @@ static int snmp_agent_config_table_index_oid(table_definition_t *td,
     ERROR(PLUGIN_NAME ": Failed to parse table index OID (%s)",
           ci->values[0].value.string);
     td->index_oid.oid_len = 0;
-    return (-EINVAL);
+    return -EINVAL;
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_config_table_data(table_definition_t *td,
@@ -845,13 +945,13 @@ static int snmp_agent_config_table_data(table_definition_t *td,
   dd = calloc(1, sizeof(*dd));
   if (dd == NULL) {
     ERROR(PLUGIN_NAME ": Failed to allocate memory for table data definition");
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   ret = cf_util_get_string(ci, &dd->name);
   if (ret != 0) {
     sfree(dd);
-    return (-1);
+    return -1;
   }
 
   dd->scale = 1.0;
@@ -885,19 +985,19 @@ static int snmp_agent_config_table_data(table_definition_t *td,
 
     if (ret != 0) {
       snmp_agent_free_data(&dd);
-      return (-1);
+      return -1;
     }
   }
 
   llentry_t *entry = llentry_create(dd->name, dd);
   if (entry == NULL) {
     snmp_agent_free_data(&dd);
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   llist_append(td->columns, entry);
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_config_data(oconfig_item_t *ci) {
@@ -909,13 +1009,13 @@ static int snmp_agent_config_data(oconfig_item_t *ci) {
   dd = calloc(1, sizeof(*dd));
   if (dd == NULL) {
     ERROR(PLUGIN_NAME ": Failed to allocate memory for data definition");
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   ret = cf_util_get_string(ci, &dd->name);
   if (ret != 0) {
     free(dd);
-    return (-1);
+    return -1;
   }
 
   dd->scale = 1.0;
@@ -947,29 +1047,29 @@ static int snmp_agent_config_data(oconfig_item_t *ci) {
 
     if (ret != 0) {
       snmp_agent_free_data(&dd);
-      return (-1);
+      return -1;
     }
   }
 
   llentry_t *entry = llentry_create(dd->name, dd);
   if (entry == NULL) {
     snmp_agent_free_data(&dd);
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   llist_append(g_agent->scalars, entry);
 
-  return (0);
+  return 0;
 }
 
 static int num_compare(const int *a, const int *b) {
   assert((a != NULL) && (b != NULL));
   if (*a < *b)
-    return (-1);
+    return -1;
   else if (*a > *b)
-    return (1);
+    return 1;
   else
-    return (0);
+    return 0;
 }
 
 static int snmp_agent_config_table(oconfig_item_t *ci) {
@@ -981,20 +1081,20 @@ static int snmp_agent_config_table(oconfig_item_t *ci) {
   td = calloc(1, sizeof(*td));
   if (td == NULL) {
     ERROR(PLUGIN_NAME ": Failed to allocate memory for table definition");
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   ret = cf_util_get_string(ci, &td->name);
   if (ret != 0) {
     sfree(td);
-    return (-1);
+    return -1;
   }
 
   td->columns = llist_create();
   if (td->columns == NULL) {
     ERROR(PLUGIN_NAME ": Failed to allocate memory for columns list");
     snmp_agent_free_table(&td);
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   for (int i = 0; i < ci->children_num; i++) {
@@ -1013,33 +1113,32 @@ static int snmp_agent_config_table(oconfig_item_t *ci) {
 
     if (ret != 0) {
       snmp_agent_free_table(&td);
-      return (-ENOMEM);
+      return -ENOMEM;
     }
   }
 
-  llentry_t *entry = llentry_create(td->name, td);
-  if (entry == NULL) {
-    snmp_agent_free_table(&td);
-    return (-ENOMEM);
-  }
-
   td->instance_index =
       c_avl_create((int (*)(const void *, const void *))strcmp);
   if (td->instance_index == NULL) {
     snmp_agent_free_table(&td);
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   td->index_instance =
       c_avl_create((int (*)(const void *, const void *))num_compare);
   if (td->index_instance == NULL) {
     snmp_agent_free_table(&td);
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
+  llentry_t *entry = llentry_create(td->name, td);
+  if (entry == NULL) {
+    snmp_agent_free_table(&td);
+    return -ENOMEM;
+  }
   llist_append(g_agent->tables, entry);
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_get_value_from_ds_type(const value_t *val, int type,
@@ -1062,10 +1161,10 @@ static int snmp_agent_get_value_from_ds_type(const value_t *val, int type,
     break;
   default:
     ERROR(PLUGIN_NAME ": Unknown data source type: %i", type);
-    return (-EINVAL);
+    return -EINVAL;
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_set_vardata(void *data, size_t *data_len, u_char asn_type,
@@ -1091,13 +1190,13 @@ static int snmp_agent_set_vardata(void *data, size_t *data_len, u_char asn_type,
   case ASN_TIMETICKS:
   case ASN_GAUGE:
     if (*data_len < sizeof(*var.integer))
-      return (-EINVAL);
+      return -EINVAL;
     *var.integer = new_value;
     *data_len = sizeof(*var.integer);
     break;
   case ASN_COUNTER64:
     if (*data_len < sizeof(*var.counter64))
-      return (-EINVAL);
+      return -EINVAL;
     var.counter64->high = (u_long)((int64_t)new_value >> 32);
     var.counter64->low = (u_long)((int64_t)new_value & 0xFFFFFFFF);
     *data_len = sizeof(*var.counter64);
@@ -1107,22 +1206,22 @@ static int snmp_agent_set_vardata(void *data, size_t *data_len, u_char asn_type,
       char buf[DATA_MAX_NAME_LEN];
       snprintf(buf, sizeof(buf), "%.2f", val->gauge);
       if (*data_len < strlen(buf))
-        return (-EINVAL);
+        return -EINVAL;
       *data_len = strlen(buf);
       memcpy(var.string, buf, *data_len);
     } else {
       ERROR(PLUGIN_NAME ": Failed to convert %d ds type to %d asn type", type,
             asn_type);
-      return (-EINVAL);
+      return -EINVAL;
     }
     break;
   default:
     ERROR(PLUGIN_NAME ": Failed to convert %d ds type to %d asn type", type,
           asn_type);
-    return (-EINVAL);
+    return -EINVAL;
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_register_oid_index(oid_t *oid, int index,
@@ -1144,81 +1243,93 @@ static int snmp_agent_update_index(table_definition_t *td,
                                    const char *instance) {
 
   if (c_avl_get(td->instance_index, instance, NULL) == 0)
-    return (0);
+    return 0;
 
   int ret;
-  int *index;
+  int *index = NULL;
   char *ins;
 
   ins = strdup(instance);
   if (ins == NULL)
-    return (-ENOMEM);
-
-  index = calloc(1, sizeof(*index));
-  if (index == NULL) {
-    sfree(ins);
-    return (-ENOMEM);
-  }
+    return -ENOMEM;
 
-  *index = c_avl_size(td->instance_index) + 1;
-
-  ret = c_avl_insert(td->instance_index, ins, index);
-  if (ret != 0) {
-    sfree(ins);
-    sfree(index);
-    return ret;
-  }
+  /* need to generate index for the table */
+  if (td->index_oid.oid_len) {
+    index = calloc(1, sizeof(*index));
+    if (index == NULL) {
+      sfree(ins);
+      return -ENOMEM;
+    }
 
-  ret = c_avl_insert(td->index_instance, index, ins);
-  if (ret < 0) {
-    DEBUG(PLUGIN_NAME ": Failed to update index_instance for '%s' table",
-          td->name);
-    return ret;
-  }
+    *index = c_avl_size(td->instance_index) + 1;
 
-  DEBUG(PLUGIN_NAME ": Updated index for '%s' table [%d, %s]", td->name, *index,
-        ins);
+    ret = c_avl_insert(td->instance_index, ins, index);
+    if (ret != 0) {
+      sfree(ins);
+      sfree(index);
+      return ret;
+    }
 
-  /* need to generate index for the table */
-  if (td->index_oid.oid_len) {
+    ret = c_avl_insert(td->index_instance, index, ins);
+    if (ret < 0) {
+      DEBUG(PLUGIN_NAME ": Failed to update index_instance for '%s' table",
+            td->name);
+      c_avl_remove(td->instance_index, ins, NULL, (void **)&index);
+      sfree(ins);
+      sfree(index);
+      return ret;
+    }
 
     ret = snmp_agent_register_oid_index(&td->index_oid, *index,
                                         snmp_agent_table_index_oid_handler);
     if (ret != 0)
       return ret;
+  } else {
+    /* instance as a key is required for any table */
+    ret = c_avl_insert(td->instance_index, ins, ins);
+    if (ret != 0) {
+      sfree(ins);
+      return ret;
+    }
+  }
 
-    /* register new oids for all columns */
-    for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) {
-      data_definition_t *dd = de->value;
+  /* register new oids for all columns */
+  for (llentry_t *de = llist_head(td->columns); de != NULL; de = de->next) {
+    data_definition_t *dd = de->value;
 
-      for (int i = 0; i < dd->oids_len; i++) {
+    for (size_t i = 0; i < dd->oids_len; i++) {
+      if (td->index_oid.oid_len) {
         ret = snmp_agent_register_oid_index(&dd->oids[i], *index,
                                             snmp_agent_table_oid_handler);
-        if (ret != 0)
-          return ret;
+      } else {
+        ret = snmp_agent_register_oid_string(&dd->oids[i], ins,
+                                             snmp_agent_table_oid_handler);
       }
+
+      if (ret != 0)
+        return ret;
     }
   }
 
+  DEBUG(PLUGIN_NAME ": Updated index for '%s' table [%d, %s]", td->name,
+        (index != NULL) ? *index : -1, ins);
+
   notification_t n = {
-    .severity = NOTIF_OKAY,
-    .time = cdtime(),
-    .plugin = PLUGIN_NAME
-  };
+      .severity = NOTIF_OKAY, .time = cdtime(), .plugin = PLUGIN_NAME};
   sstrncpy(n.host, hostname_g, sizeof(n.host));
   sstrncpy(n.plugin_instance, ins, sizeof(n.plugin_instance));
-  ssnprintf(n.message, sizeof(n.message),
-            "Data row added to table %s instance %s index %d", td->name, ins,
-            *index);
+  snprintf(n.message, sizeof(n.message),
+           "Data row added to table %s instance %s index %d", td->name, ins,
+           (index != NULL) ? *index : -1);
   plugin_dispatch_notification(&n);
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_write(value_list_t const *vl) {
 
   if (vl == NULL)
-    return (-EINVAL);
+    return -EINVAL;
 
   for (llentry_t *te = llist_head(g_agent->tables); te != NULL; te = te->next) {
     table_definition_t *td = te->value;
@@ -1230,13 +1341,13 @@ static int snmp_agent_write(value_list_t const *vl) {
         if (CHECK_DD_TYPE(dd, vl->plugin, vl->plugin_instance, vl->type,
                           vl->type_instance)) {
           snmp_agent_update_index(td, vl->plugin_instance);
-          return (0);
+          return 0;
         }
       }
     }
   }
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_collect(const data_set_t *ds, const value_list_t *vl,
@@ -1248,31 +1359,40 @@ static int snmp_agent_collect(const data_set_t *ds, const value_list_t *vl,
 
   pthread_mutex_unlock(&g_agent->lock);
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_preinit(void) {
   if (g_agent != NULL) {
     /* already initialized if config callback was called before init callback */
-    return (0);
+    return 0;
   }
 
   g_agent = calloc(1, sizeof(*g_agent));
   if (g_agent == NULL) {
     ERROR(PLUGIN_NAME ": Failed to allocate memory for snmp agent context");
-    return (-ENOMEM);
+    return -ENOMEM;
   }
 
   g_agent->tables = llist_create();
   g_agent->scalars = llist_create();
 
+  if (g_agent->tables == NULL || g_agent->scalars == NULL) {
+    ERROR(PLUGIN_NAME ": llist_create() failed");
+    llist_destroy(g_agent->scalars);
+    llist_destroy(g_agent->tables);
+    return -ENOMEM;
+  }
+
   int err;
   /* make us a agentx client. */
   err = netsnmp_ds_set_boolean(NETSNMP_DS_APPLICATION_ID, NETSNMP_DS_AGENT_ROLE,
                                1);
   if (err != 0) {
     ERROR(PLUGIN_NAME ": Failed to set agent role (%d)", err);
-    return (-1);
+    llist_destroy(g_agent->scalars);
+    llist_destroy(g_agent->tables);
+    return -1;
   }
 
   /*
@@ -1283,22 +1403,28 @@ static int snmp_agent_preinit(void) {
   err = init_agent(PLUGIN_NAME);
   if (err != 0) {
     ERROR(PLUGIN_NAME ": Failed to initialize the agent library (%d)", err);
-    return (-1);
+    llist_destroy(g_agent->scalars);
+    llist_destroy(g_agent->tables);
+    return -1;
   }
 
   init_snmp(PLUGIN_NAME);
 
   g_agent->tp = read_all_mibs();
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_init(void) {
   int ret;
 
-  ret = snmp_agent_preinit();
-  if (ret != 0)
-    return ret;
+  if (g_agent == NULL || ((llist_head(g_agent->scalars) == NULL) &&
+                          (llist_head(g_agent->tables) == NULL))) {
+    ERROR(PLUGIN_NAME ": snmp_agent_init: plugin not configured");
+    return -EINVAL;
+  }
+
+  plugin_register_shutdown(PLUGIN_NAME, snmp_agent_shutdown);
 
   ret = snmp_agent_register_scalar_oids();
   if (ret != 0)
@@ -1308,13 +1434,6 @@ static int snmp_agent_init(void) {
   if (ret != 0)
     return ret;
 
-  /* create a second thread to listen for requests from AgentX*/
-  ret = pthread_create(&g_agent->thread, NULL, &snmp_agent_thread_run, NULL);
-  if (ret != 0) {
-    ERROR(PLUGIN_NAME ": Failed to create a separate thread, err %u", ret);
-    return ret;
-  }
-
   ret = pthread_mutex_init(&g_agent->lock, NULL);
   if (ret != 0) {
     ERROR(PLUGIN_NAME ": Failed to initialize mutex, err %u", ret);
@@ -1327,7 +1446,19 @@ static int snmp_agent_init(void) {
     return ret;
   }
 
-  return (0);
+  /* create a second thread to listen for requests from AgentX*/
+  ret = pthread_create(&g_agent->thread, NULL, &snmp_agent_thread_run, NULL);
+  if (ret != 0) {
+    ERROR(PLUGIN_NAME ": Failed to create a separate thread, err %u", ret);
+    return ret;
+  }
+
+  if (llist_head(g_agent->tables) != NULL) {
+    plugin_register_write(PLUGIN_NAME, snmp_agent_collect, NULL);
+    plugin_register_missing(PLUGIN_NAME, snmp_agent_clear_missing, NULL);
+  }
+
+  return 0;
 }
 
 static void *snmp_agent_thread_run(void __attribute__((unused)) * arg) {
@@ -1358,7 +1489,7 @@ static int snmp_agent_register_oid(oid_t *oid, Netsnmp_Node_Handler *handler) {
     WARNING(PLUGIN_NAME
             ": Skipped registration: OID (%s) is not found in main tree",
             oid_str);
-    return (0);
+    return 0;
   }
 
   reg = netsnmp_create_handler_registration(oid_name, handler, oid->oid,
@@ -1366,7 +1497,7 @@ static int snmp_agent_register_oid(oid_t *oid, Netsnmp_Node_Handler *handler) {
   if (reg == NULL) {
     ERROR(PLUGIN_NAME ": Failed to create handler registration for OID (%s)",
           oid_str);
-    return (-1);
+    return -1;
   }
 
   pthread_mutex_lock(&g_agent->agentx_lock);
@@ -1374,20 +1505,20 @@ static int snmp_agent_register_oid(oid_t *oid, Netsnmp_Node_Handler *handler) {
   if (netsnmp_register_instance(reg) != MIB_REGISTERED_OK) {
     ERROR(PLUGIN_NAME ": Failed to register handler for OID (%s)", oid_str);
     pthread_mutex_unlock(&g_agent->agentx_lock);
-    return (-1);
+    return -1;
   }
 
   pthread_mutex_unlock(&g_agent->agentx_lock);
 
   DEBUG(PLUGIN_NAME ": Registered handler for OID (%s)", oid_str);
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_free_config(void) {
 
   if (g_agent == NULL)
-    return (-EINVAL);
+    return -EINVAL;
 
   for (llentry_t *te = llist_head(g_agent->tables); te != NULL; te = te->next)
     snmp_agent_free_table((table_definition_t **)&te->value);
@@ -1397,7 +1528,7 @@ static int snmp_agent_free_config(void) {
     snmp_agent_free_data((data_definition_t **)&de->value);
   llist_destroy(g_agent->scalars);
 
-  return (0);
+  return 0;
 }
 
 static int snmp_agent_shutdown(void) {
@@ -1407,7 +1538,7 @@ static int snmp_agent_shutdown(void) {
 
   if (g_agent == NULL) {
     ERROR(PLUGIN_NAME ": snmp_agent_shutdown: plugin not initialized");
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   if (pthread_cancel(g_agent->thread) != 0)
@@ -1434,7 +1565,7 @@ static int snmp_agent_config(oconfig_item_t *ci) {
 
   if (ret != 0) {
     sfree(g_agent);
-    return (-EINVAL);
+    return -EINVAL;
   }
 
   for (int i = 0; i < ci->children_num; i++) {
@@ -1453,7 +1584,7 @@ static int snmp_agent_config(oconfig_item_t *ci) {
       snmp_agent_free_config();
       snmp_shutdown(PLUGIN_NAME);
       sfree(g_agent);
-      return (-EINVAL);
+      return -EINVAL;
     }
   }
 
@@ -1463,16 +1594,13 @@ static int snmp_agent_config(oconfig_item_t *ci) {
     snmp_agent_free_config();
     snmp_shutdown(PLUGIN_NAME);
     sfree(g_agent);
-    return (-EINVAL);
+    return -EINVAL;
   }
 
-  return (0);
+  return 0;
 }
 
 void module_register(void) {
   plugin_register_init(PLUGIN_NAME, snmp_agent_init);
   plugin_register_complex_config(PLUGIN_NAME, snmp_agent_config);
-  plugin_register_write(PLUGIN_NAME, snmp_agent_collect, NULL);
-  plugin_register_missing(PLUGIN_NAME, snmp_agent_clear_missing, NULL);
-  plugin_register_shutdown(PLUGIN_NAME, snmp_agent_shutdown);
 }