Merge pull request #3329 from efuss/fix-3311
[collectd.git] / src / snmp_agent.c
index ef03710..bb4a7e4 100644 (file)
@@ -1,7 +1,7 @@
 /**
  * collectd - src/snmp_agent.c
  *
- * Copyright(c) 2017 Intel Corporation. All rights reserved.
+ * Copyright(c) 2017-2018 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
@@ -29,8 +29,8 @@
 
 #include "collectd.h"
 
-#include "common.h"
-#include "utils_avltree.h"
+#include "utils/avltree/avltree.h"
+#include "utils/common/common.h"
 #include "utils_cache.h"
 #include "utils_llist.h"
 #include <regex.h>
@@ -101,7 +101,7 @@ struct table_definition_s {
                                              will be split into sepearate
                                              tokens */
 
-  _Bool tokens_done; /* Set to 1 when all tokens are generated */
+  bool tokens_done; /* Set to true when all tokens are generated */
 };
 typedef struct table_definition_s table_definition_t;
 
@@ -134,7 +134,7 @@ struct snmp_agent_ctx_s {
 typedef struct snmp_agent_ctx_s snmp_agent_ctx_t;
 
 static snmp_agent_ctx_t *g_agent;
-const char *const index_opts[MAX_KEY_SOURCES] = {
+static const char *index_opts[MAX_KEY_SOURCES] = {
     "Hostname", "Plugin", "PluginInstance", "Type", "TypeInstance"};
 
 #define CHECK_DD_TYPE(_dd, _p, _pi, _t, _ti)                                   \
@@ -172,7 +172,7 @@ 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++) {
-    snprintf(oid_str[i], sizeof(oid_str[i]), "%lu", (unsigned long)o->oid[i]);
+    ssnprintf(oid_str[i], sizeof(oid_str[i]), "%lu", (unsigned long)o->oid[i]);
     oid_str_ptr[i] = oid_str[i];
   }
 
@@ -383,11 +383,19 @@ static int snmp_agent_create_token(char const *input, int t_off, int n,
   int ret = 0;
 
   token->key = index_key;
-  token->str = strndup(input + t_off, n);
+  input += t_off;
+  size_t len = strlen(input);
+
+  if (n < len)
+    len = n;
+
+  token->str = malloc(len + 1);
 
   if (token->str == NULL)
     goto free_offset_error;
 
+  /* copy at most n bytes from input with offset t_off into token->str */
+  sstrncpy(token->str, input, len + 1);
   *offset = t_off;
   ret = c_avl_insert(tree, (void *)offset, (void *)token);
 
@@ -493,7 +501,6 @@ static int snmp_agent_fill_index_list(table_definition_t *td,
     /* var should never be NULL */
     assert(key != NULL);
     ptr = NULL;
-    ret = 0;
     const index_key_src_t source = td->index_keys[i].source;
     c_avl_tree_t *const tokens = td->tokens[source];
     /* Generating list filled with all data necessary to generate an OID */
@@ -517,12 +524,10 @@ static int snmp_agent_fill_index_list(table_definition_t *td,
       ERROR(PLUGIN_NAME ": Unknown index key source provided");
       return -EINVAL;
     }
-    if (ret != 0)
-      return -EINVAL;
 
     /* Parsing input string if necessary */
     if (td->index_keys[i].regex) {
-      regmatch_t m = {-1, -1};
+      regmatch_t m;
 
       /* Parsing input string */
       ret = snmp_agent_parse_index_key(ptr, &td->index_keys[i].regex_info,
@@ -533,7 +538,7 @@ static int snmp_agent_fill_index_list(table_definition_t *td,
       }
 
       /* Tokenizing input string if not done yet */
-      if (td->tokens_done == 0)
+      if (td->tokens_done == false)
         ret = snmp_agent_tokenize(ptr, tokens, &m, key);
 
       if (ret != 0)
@@ -560,12 +565,16 @@ static int snmp_agent_fill_index_list(table_definition_t *td,
 #else
       ret = snmp_set_var_value(key, ptr, strlen(ptr));
 #endif
+
+    if (ret != 0)
+      return -1;
+
     key = key->next_variable;
   }
 
   /* Tokens for all source strings are generated */
   for (i = 0; i < MAX_KEY_SOURCES; i++)
-    td->tokens_done = 1;
+    td->tokens_done = true;
 
   return 0;
 }
@@ -727,13 +736,14 @@ static void snmp_agent_table_data_remove(data_definition_t *dd,
   if (index == NULL)
     snmp_agent_oid_to_string(index_str, sizeof(index_str), index_oid);
   else
-    snprintf(index_str, sizeof(index_str), "%d", *index);
+    ssnprintf(index_str, sizeof(index_str), "%d", *index);
 
   notification_t n = {
       .severity = NOTIF_WARNING, .time = cdtime(), .plugin = PLUGIN_NAME};
   sstrncpy(n.host, hostname_g, sizeof(n.host));
-  snprintf(n.message, sizeof(n.message),
-           "Removed data row from table %s with index %s", td->name, index_str);
+  ssnprintf(n.message, sizeof(n.message),
+            "Removed data row from table %s with index %s", td->name,
+            index_str);
   DEBUG(PLUGIN_NAME ": %s", n.message);
   plugin_dispatch_notification(&n);
 
@@ -927,6 +937,7 @@ static void snmp_agent_free_table(table_definition_t **td) {
 
 static int snmp_agent_parse_oid_index_keys(const table_definition_t *td,
                                            oid_t *index_oid) {
+  assert(index_oid != NULL);
   int ret = parse_oid_indexes(index_oid->oid, index_oid->oid_len,
                               td->index_list_cont);
   if (ret != SNMPERR_SUCCESS)
@@ -935,7 +946,6 @@ static int snmp_agent_parse_oid_index_keys(const table_definition_t *td,
 }
 
 static int snmp_agent_build_name(char **name, c_avl_tree_t *tokens) {
-
   int *pos;
   token_t *tok;
   char str[DATA_MAX_NAME_LEN];
@@ -948,19 +958,19 @@ static int snmp_agent_build_name(char **name, c_avl_tree_t *tokens) {
   }
 
   while (c_avl_iterator_next(it, (void **)&pos, (void **)&tok) == 0) {
-    strncat(out, tok->str, strlen(tok->str));
+    strncat(out, tok->str, DATA_MAX_NAME_LEN - strlen(out) - 1);
     if (tok->key != NULL) {
       if (tok->key->type == ASN_INTEGER) {
-        snprintf(str, sizeof(str), "%ld", *tok->key->val.integer);
-        strncat(out, str, strlen(str));
-      } else /* OCTET_STR */
+        ssnprintf(str, sizeof(str), "%ld", *tok->key->val.integer);
+        strncat(out, str, DATA_MAX_NAME_LEN - strlen(out) - 1);
+      } else /* OCTET_STR */
         strncat(out, (char *)tok->key->val.string,
-                strlen((char *)tok->key->val.string));
-      }
+                DATA_MAX_NAME_LEN - strlen(out) - 1);
     }
   }
-  *name = strdup(out);
+
   c_avl_iterator_destroy(it);
+  *name = strdup(out);
 
   if (*name == NULL) {
     ERROR(PLUGIN_NAME ": Could not allocate memory");
@@ -1004,7 +1014,7 @@ static int snmp_agent_format_name(char *name, int name_len,
         }
 
         if (td->index_keys[i].type == ASN_INTEGER) {
-          snprintf(str, sizeof(str), "%ld", *key->val.integer);
+          ssnprintf(str, sizeof(str), "%ld", *key->val.integer);
           fields[source] = str;
         } else /* OCTET_STR */
           fields[source] = (char *)key->val.string;
@@ -1500,7 +1510,7 @@ static int snmp_agent_config_index_key_source(table_definition_t *td,
   if (ret != 0)
     return -1;
 
-  _Bool match = 0;
+  bool match = false;
 
   for (int i = 0; i < MAX_KEY_SOURCES; i++) {
     if (strcasecmp(index_opts[i], (const char *)val) == 0) {
@@ -1520,7 +1530,7 @@ static int snmp_agent_config_index_key_source(table_definition_t *td,
 
   sfree(val);
   dd->index_key_pos = td->index_keys_len++;
-  dd->is_index_key = 1;
+  dd->is_index_key = true;
 
   return 0;
 }
@@ -1599,14 +1609,14 @@ static int snmp_agent_config_table_column(table_definition_t *td,
   dd->shift = 0.0;
   /* NULL if it's a scalar */
   dd->table = td;
-  dd->is_index_key = 0;
+  dd->is_index_key = false;
 
   for (int i = 0; i < ci->children_num; i++) {
     oconfig_item_t *option = ci->children + i;
 
     /* First 3 options are reserved for table entry only */
     if (td != NULL && strcasecmp("IndexKey", option->key) == 0) {
-      dd->is_index_key = 1;
+      dd->is_index_key = true;
       option_tmp = option;
     } else if (strcasecmp("Plugin", option->key) == 0)
       ret = cf_util_get_string(option, &dd->plugin);
@@ -1705,7 +1715,7 @@ static int snmp_agent_config_table(oconfig_item_t *ci) {
 
   for (int i = 0; i < MAX_KEY_SOURCES; i++)
     td->tokens[i] = NULL;
-  td->tokens_done = 0;
+  td->tokens_done = false;
 
   for (int i = 0; i < ci->children_num; i++) {
     oconfig_item_t *option = ci->children + i;
@@ -1827,7 +1837,7 @@ static int snmp_agent_set_vardata(void *data, size_t *data_len, u_char asn_type,
   case ASN_OCTET_STR:
     if (type == DS_TYPE_GAUGE) {
       char buf[DATA_MAX_NAME_LEN];
-      snprintf(buf, sizeof(buf), "%.2f", val->gauge);
+      ssnprintf(buf, sizeof(buf), "%.2f", val->gauge);
       if (*data_len < strlen(buf))
         return -EINVAL;
       *data_len = strlen(buf);
@@ -1876,31 +1886,31 @@ static int snmp_agent_update_instance_oids(c_avl_tree_t *tree, oid_t *index_oid,
 }
 
 static int snmp_agent_update_index(data_definition_t *dd,
-                                   table_definition_t *td, oid_t **index_oid) {
+                                   table_definition_t *td, oid_t *index_oid,
+                                   bool *free_index_oid) {
   int ret;
   int *index = NULL;
   int *value = NULL;
-  _Bool free_index_oid = 1;
 
-  if (c_avl_get(td->instance_index, (void *)*index_oid, (void **)&index) != 0) {
-    /* Processing new instance */
-    free_index_oid = 0;
+  if (c_avl_get(td->instance_index, (void *)index_oid, (void **)&index) != 0) {
+    /* We'll keep index_oid stored in AVL tree */
+    *free_index_oid = false;
 
     /* need to generate index for the table */
     if (td->index_oid.oid_len) {
       index = calloc(1, sizeof(*index));
       if (index == NULL) {
         ret = -ENOMEM;
-        goto free_index_oid;
+        goto error;
       }
 
       *index = c_avl_size(td->instance_index) + 1;
 
-      ret = c_avl_insert(td->instance_index, *index_oid, index);
+      ret = c_avl_insert(td->instance_index, index_oid, index);
       if (ret != 0)
         goto free_index;
 
-      ret = c_avl_insert(td->index_instance, index, *index_oid);
+      ret = c_avl_insert(td->index_instance, index, index_oid);
       if (ret < 0) {
         DEBUG(PLUGIN_NAME ": Failed to update index_instance for '%s' table",
               td->name);
@@ -1913,9 +1923,9 @@ static int snmp_agent_update_index(data_definition_t *dd,
         goto remove_avl_index;
     } else {
       /* instance as a key is required for any table */
-      ret = c_avl_insert(td->instance_index, *index_oid, NULL);
+      ret = c_avl_insert(td->instance_index, index_oid, NULL);
       if (ret != 0)
-        goto free_index_oid;
+        goto error;
     }
 
     value = calloc(1, sizeof(*value));
@@ -1926,7 +1936,7 @@ static int snmp_agent_update_index(data_definition_t *dd,
       goto unregister_index;
     }
 
-    ret = c_avl_insert(td->instance_oids, *index_oid, value);
+    ret = c_avl_insert(td->instance_oids, index_oid, value);
 
     if (ret < 0) {
       DEBUG(PLUGIN_NAME ": Failed to update instance_oids for '%s' table",
@@ -1947,7 +1957,7 @@ static int snmp_agent_update_index(data_definition_t *dd,
           ret = snmp_agent_register_oid_index(&idd->oids[i], *index,
                                               snmp_agent_table_oid_handler);
         else
-          ret = snmp_agent_register_oid_string(&idd->oids[i], *index_oid,
+          ret = snmp_agent_register_oid_string(&idd->oids[i], index_oid,
                                                snmp_agent_table_oid_handler);
 
         if (ret != 0) {
@@ -1968,14 +1978,14 @@ static int snmp_agent_update_index(data_definition_t *dd,
       ret = snmp_agent_register_oid_index(&dd->oids[i], *index,
                                           snmp_agent_table_oid_handler);
     else
-      ret = snmp_agent_register_oid_string(&dd->oids[i], *index_oid,
+      ret = snmp_agent_register_oid_string(&dd->oids[i], index_oid,
                                            snmp_agent_table_oid_handler);
 
     if (ret < 0)
       goto free_index;
     else if (ret == OID_EXISTS)
       break;
-    else if (snmp_agent_update_instance_oids(td->instance_oids, *index_oid, 1) <
+    else if (snmp_agent_update_instance_oids(td->instance_oids, index_oid, 1) <
              0)
       goto free_index;
   }
@@ -1984,40 +1994,37 @@ static int snmp_agent_update_index(data_definition_t *dd,
     char index_str[DATA_MAX_NAME_LEN];
 
     if (index == NULL)
-      snmp_agent_oid_to_string(index_str, sizeof(index_str), *index_oid);
+      snmp_agent_oid_to_string(index_str, sizeof(index_str), index_oid);
     else
-      snprintf(index_str, sizeof(index_str), "%d", *index);
+      ssnprintf(index_str, sizeof(index_str), "%d", *index);
 
     notification_t n = {
         .severity = NOTIF_OKAY, .time = cdtime(), .plugin = PLUGIN_NAME};
     sstrncpy(n.host, hostname_g, sizeof(n.host));
-    snprintf(n.message, sizeof(n.message),
-             "Data added to table %s with index %s", td->name, index_str);
+    ssnprintf(n.message, sizeof(n.message),
+              "Data added to table %s with index %s", td->name, index_str);
     DEBUG(PLUGIN_NAME ": %s", n.message);
 
     plugin_dispatch_notification(&n);
   }
 
-  if (free_index_oid)
-    sfree(*index_oid);
-
   return 0;
 
 free_value:
   sfree(value);
 unregister_index:
   if (td->index_oid.oid_len)
-    snmp_agent_unregister_oid_index(*index_oid, *index);
+    snmp_agent_unregister_oid_index(index_oid, *index);
 remove_avl_index:
   if (td->index_oid.oid_len)
     c_avl_remove(td->index_instance, index, NULL, NULL);
 remove_avl_index_oid:
-  c_avl_remove(td->instance_index, *index_oid, NULL, NULL);
+  c_avl_remove(td->instance_index, index_oid, NULL, NULL);
 free_index:
   if (index != NULL)
     sfree(index);
-free_index_oid:
-  sfree(*index_oid);
+error:
+  *free_index_oid = true;
 
   return ret;
 }
@@ -2036,6 +2043,7 @@ 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)) {
           oid_t *index_oid = calloc(1, sizeof(*index_oid));
+          bool free_index_oid = true;
 
           if (index_oid == NULL) {
             ERROR(PLUGIN_NAME ": Could not allocate memory for index_oid");
@@ -2045,7 +2053,11 @@ static int snmp_agent_write(value_list_t const *vl) {
           int ret = snmp_agent_generate_index(td, vl, index_oid);
 
           if (ret == 0)
-            ret = snmp_agent_update_index(dd, td, &index_oid);
+            ret = snmp_agent_update_index(dd, td, index_oid, &free_index_oid);
+
+          /* Index exists or update failed */
+          if (free_index_oid)
+            sfree(index_oid);
 
           return ret;
         }
@@ -2191,8 +2203,7 @@ static int snmp_agent_register_oid(oid_t *oid, Netsnmp_Node_Handler *handler) {
   if (c_avl_get(g_agent->registered_oids, (void *)oid, NULL) == 0)
     return OID_EXISTS;
   else {
-    oid_t *new_oid = calloc(1, sizeof(*oid));
-
+    oid_t *new_oid = calloc(1, sizeof(*new_oid));
     if (new_oid == NULL) {
       ERROR(PLUGIN_NAME ": Could not allocate memory to register new OID");
       return -ENOMEM;