[collectd 6] Fix some gcc warnings with more strict checks (#3970)

* Remove unused dummy meta data functions

"format_stackdriver" was migrated over year ago.

* Fix rest of the GCC warnings from collectd core

* Properly initialize complex struct
* Fix signed vs unsigned comparisons
* Tell compiler which args are expected to be unused

Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.

* write_prometheus: fix static analysis warnings and comments

* Fix unused arguments reported by:
  "-O3 -Werror -Wall -Wextra -Wformat-security"
* Fix obsolete comment to match MHD docs:
  https://www.gnu.org/software/libmicrohttpd/ ("Queueing responses" section)
  https://git.gnunet.org/libmicrohttpd.git/tree/src/include/microhttpd.h#n2398
* Fix use after free reported by Klocwork, "prom_fam" cannot be used
  after it's been freed

* Fix signedness mismatch GCC warnings in few of the plugins

Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.

* Remove unused function arguments from few plugins

Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.

* Attribute unused functions arguments as such in few of the plugins

Based on "-O3 -Werror -Wall -Wextra -Wformat-security" output.

* turbostat: Satisfy clang-format CI check

Apparently CI has changed since this code was added to collectd.

Co-authored-by: Matthias Runge <mrunge@redhat.com>

diff --git a/src/cgroups.c b/src/cgroups.c
index 8925239848f0ba2af42d50e92b6a5224fc050540..2831253f714aef57eaeb81e751b8075c68ad57ef 100644 (file)
--- a/src/cgroups.c
+++ b/src/cgroups.c
@@ -52,7 +52,7 @@ cgroups_submit_one(char const *plugin_instance, char const *type_instance,
  * This callback reads the user/system CPU time for each cgroup.
  */
 static int read_cpuacct_procs(const char *dirname, char const *cgroup_name,
-                              void *user_data) {
+                              __attribute__((unused)) void *user_data) {
   char abs_path[PATH_MAX];
   struct stat statbuf;
   char buf[1024];
@@ -131,7 +131,7 @@ static int read_cpuacct_procs(const char *dirname, char const *cgroup_name,
  * read_cpuacct_procs callback on every folder it finds, such as "system".
  */
 static int read_cpuacct_root(const char *dirname, const char *filename,
-                             void *user_data) {
+                             __attribute__((unused)) void *user_data) {
   char abs_path[PATH_MAX];
   struct stat statbuf;
   int status;

diff --git a/src/conntrack.c b/src/conntrack.c
index 7b61eef0814647df272bde241244f9468164716d..867b3cb6c1733e50649998a01324cdf540923e75 100644 (file)
--- a/src/conntrack.c
+++ b/src/conntrack.c
@@ -43,7 +43,8 @@ static int config_keys_num = STATIC_ARRAY_SIZE(config_keys);
 
 static int old_files;
 
-static int conntrack_config(const char *key, const char *value) {
+static int conntrack_config(const char *key,
+                            __attribute__((unused)) const char *value) {
   if (strcmp(key, "OldFiles") == 0)
     old_files = 1;
 

diff --git a/src/daemon/types_list.c b/src/daemon/types_list.c
index f5eff1a2a01169748671e7a3518d4cfdae784e13..03b4f16e62df1e193746fe63a9bb44f1d4a2518e 100644 (file)
--- a/src/daemon/types_list.c
+++ b/src/daemon/types_list.c
@@ -101,7 +101,7 @@ static void parse_line(char *buf) {
   if (fields[0][0] == '#')
     return;
 
-  data_set_t ds = {{0}};
+  data_set_t ds = {{0}, 0, NULL};
 
   sstrncpy(ds.type, fields[0], sizeof(ds.type));
 

diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c
index b983fdd9aaad4408c67f2af222949baa9a023f09..b9c64ef5141c20b256f5b1ce101ace3d5a4acd7f 100644 (file)
--- a/src/daemon/utils_cache.c
+++ b/src/daemon/utils_cache.c
@@ -995,23 +995,3 @@ int uc_meta_data_get_double(metric_t const *m, const char *key, double *value)
 int uc_meta_data_get_boolean(metric_t const *m, const char *key, bool *value)
     UC_WRAP(meta_data_get_boolean);
 #undef UC_WRAP
-
-int uc_meta_data_get_signed_int_vl(value_list_t const *vl, char const *key,
-                                   int64_t *value) {
-  return ENOTSUP;
-}
-
-int uc_meta_data_get_unsigned_int_vl(value_list_t const *vl, char const *key,
-                                     uint64_t *value) {
-  return ENOTSUP;
-}
-
-int uc_meta_data_add_signed_int_vl(value_list_t const *vl, char const *key,
-                                   int64_t value) {
-  return ENOTSUP;
-}
-
-int uc_meta_data_add_unsigned_int_vl(value_list_t const *vl, char const *key,
-                                     uint64_t value) {
-  return ENOTSUP;
-}

diff --git a/src/daemon/utils_cache.h b/src/daemon/utils_cache.h
index 514eab55880da55c9c3b36f17e2654ae445a4879..8ae223b088d6311d44595223ee85da13037a19fc 100644 (file)
--- a/src/daemon/utils_cache.h
+++ b/src/daemon/utils_cache.h
@@ -142,15 +142,4 @@ int uc_meta_data_get_unsigned_int(metric_t const *m, const char *key,
 int uc_meta_data_get_double(metric_t const *m, const char *key, double *value);
 int uc_meta_data_get_boolean(metric_t const *m, const char *key, bool *value);
 
-/* TODO(octo): Remove these dummy functions after format_stackdriver has been
- * migrated. */
-int uc_meta_data_get_signed_int_vl(value_list_t const *vl, char const *key,
-                                   int64_t *value);
-int uc_meta_data_get_unsigned_int_vl(value_list_t const *vl, char const *key,
-                                     uint64_t *value);
-int uc_meta_data_add_signed_int_vl(value_list_t const *vl, char const *key,
-                                   int64_t value);
-int uc_meta_data_add_unsigned_int_vl(value_list_t const *vl, char const *key,
-                                     uint64_t value);
-
 #endif /* !UTILS_CACHE_H */

diff --git a/src/infiniband.c b/src/infiniband.c
index c70c824d6fec496d631fcdc17c5662ed5d9cd49c..cc5d83e525caccefbe1fc83c84505d314fe748b4 100644 (file)
--- a/src/infiniband.c
+++ b/src/infiniband.c
@@ -118,7 +118,7 @@ static int ib_read_value_file_num_only(const char *device, const char *port,
   strstripnewline(buffer);
 
   // zero-out the first non-digit character
-  for (int i = 0; i < sizeof(buffer); i++) {
+  for (unsigned i = 0; i < sizeof(buffer); i++) {
     if (!isdigit(buffer[i])) {
       buffer[i] = '\0';
       break;
@@ -327,7 +327,7 @@ static int infiniband_read(void) {
   char port_name[255];
 
   if (ib_glob_ports(&g) == 0) {
-    for (int i = 0; i < g.gl_pathc; ++i) {
+    for (unsigned i = 0; i < g.gl_pathc; ++i) {
       char *device = NULL, *port = NULL;
       if (ib_parse_glob_port(g.gl_pathv[i], &device, &port) == 0) {
         snprintf(port_name, sizeof(port_name), "%s:%s", device, port);

diff --git a/src/libcollectdclient/network_parse.c b/src/libcollectdclient/network_parse.c
index f74fd801dabba4124453c6c5a783e404c2c7420d..ad7841fa1064a05bd119fd79ab54dcb76ae7ff8d 100644 (file)
--- a/src/libcollectdclient/network_parse.c
+++ b/src/libcollectdclient/network_parse.c
@@ -385,9 +385,11 @@ static int verify_sha256(void *payload, size_t payload_size,
   return !!ret;
 }
 #else /* !HAVE_GCRYPT_H */
-static int verify_sha256(void *payload, size_t payload_size,
-                         char const *username, char const *password,
-                         uint8_t hash_provided[32]) {
+static int verify_sha256(__attribute__((unused)) void *payload,
+                         __attribute__((unused)) size_t payload_size,
+                         __attribute__((unused)) char const *username,
+                         __attribute__((unused)) char const *password,
+                         __attribute__((unused)) uint8_t hash_provided[32]) {
   return ENOTSUP;
 }
 #endif
@@ -502,7 +504,9 @@ static int parse_encrypt_aes256(void *data, size_t data_size,
   return network_parse(b->data, b->len, ENCRYPT, opts);
 }
 #else /* !HAVE_GCRYPT_H */
-static int parse_encrypt_aes256(void *data, size_t data_size,
+static int parse_encrypt_aes256(__attribute__((unused)) void *data,
+                                __attribute__((unused)) size_t data_size,
+                                __attribute__((unused))
                                 lcc_network_parse_options_t const *opts) {
   return ENOTSUP;
 }

diff --git a/src/logparser.c b/src/logparser.c
index c326ce865c83f112c61e34f118a2269e3013a729..6317d31d391db7e1c59e3d8eedd58070380a320e 100644 (file)
--- a/src/logparser.c
+++ b/src/logparser.c
@@ -401,7 +401,7 @@ static int logparser_validate_config(void) {
       return -1;
     }
 
-    for (int j = 0; j < parser->patterns_len; j++) {
+    for (unsigned j = 0; j < parser->patterns_len; j++) {
       message_pattern_t *pattern = parser->patterns + j;
 
       if (pattern->regex == NULL) {
@@ -562,7 +562,7 @@ static void logparser_process_msg(log_parser_t *parser, message_t *msg,
   if (parser->def_type_inst != NULL)
     sstrncpy(n.type_instance, parser->def_type_inst, sizeof(n.type_instance));
 
-  for (int i = 0; i < max_items; i++) {
+  for (unsigned i = 0; i < max_items; i++) {
     message_item_t *item = msg->message_items + i;
     if (!item->value[0])
       break;

diff --git a/src/statsd.c b/src/statsd.c
index 322339c7b3dcdf0ec18a2cc1489c4145bb4abd9e..df7654a03ae086e84abc1f6ee65db258dfd66609 100644 (file)
--- a/src/statsd.c
+++ b/src/statsd.c
@@ -553,7 +553,7 @@ static int statsd_network_init(struct pollfd **ret_fds, /* {{{ */
   return 0;
 } /* }}} int statsd_network_init */
 
-static void *statsd_network_thread(void *args) /* {{{ */
+static void *statsd_network_thread(__attribute__((unused)) void *args) /* {{{ */
 {
   struct pollfd *fds = NULL;
   size_t fds_num = 0;

diff --git a/src/table.c b/src/table.c
index f181de94624b2be7201819592d120c43d61b5455..223d2eebe212b0efa22e574b9245dc5ca16d6119 100644 (file)
--- a/src/table.c
+++ b/src/table.c
@@ -392,7 +392,7 @@ static int tbl_result_dispatch(tbl_t *tbl, tbl_result_t *res, char **fields,
   return 0;
 } /* tbl_result_dispatch */
 
-static int tbl_parse_line(tbl_t *tbl, char *line, size_t len) {
+static int tbl_parse_line(tbl_t *tbl, char *line) {
   char *fields[tbl->max_colnum + 1];
   size_t i = 0;
 
@@ -438,7 +438,7 @@ static int tbl_read_table(tbl_t *tbl) {
       log_warn("Table %s: Truncated line: %s", tbl->file, buf);
     }
 
-    if (tbl_parse_line(tbl, buf, sizeof(buf)) != 0) {
+    if (tbl_parse_line(tbl, buf) != 0) {
       log_warn("Table %s: Failed to parse line: %s", tbl->file, buf);
       continue;
     }

diff --git a/src/turbostat.c b/src/turbostat.c
index 8bbb92b552241a4faf40e4f880444cdbb2fe7534..09297a69974f6ba77578d96b3c00e381b5337c73 100644 (file)
--- a/src/turbostat.c
+++ b/src/turbostat.c
@@ -835,7 +835,8 @@ for_all_cpus_delta(const struct thread_data *thread_new_base,
  * Package Thermal Management Sensor (PTM), and thermal event thresholds.
  */
 static int __attribute__((warn_unused_result))
-set_temperature_target(struct thread_data *t, struct core_data *c,
+set_temperature_target(struct thread_data *t,
+                       __attribute__((unused)) struct core_data *c,
                        struct pkg_data *p) {
   unsigned long long msr;
   unsigned int target_c_local;
@@ -1463,9 +1464,9 @@ static void free_all_buffers(void) {
   package_delta = NULL;
 }
 
-  /**********************
-   * Collectd functions *
-   **********************/
+/**********************
+ * Collectd functions *
+ **********************/
 
 #define DO_OR_GOTO_ERR(something)                                              \
   do {                                                                         \

diff --git a/src/utils/cmds/flush.c b/src/utils/cmds/flush.c
index ae53b419ecc1b9693da7245e141e253cf5cfe7aa..cd3a865471d29d145245527c98b426f98aa4720c 100644 (file)
--- a/src/utils/cmds/flush.c
+++ b/src/utils/cmds/flush.c
@@ -141,10 +141,11 @@ cmd_status_t cmd_handle_flush(FILE *fh, char *buffer) {
 
       if (cmd.cmd.flush.identifiers_num != 0) {
         identifier_t *id = cmd.cmd.flush.identifiers + j;
-        if (snprintf(buf, sizeof(buf), "%s/%s/%s",
-                     (id->host == NULL) ? "" : id->host,
-                     (id->plugin == NULL) ? "" : id->plugin,
-                     (id->type == NULL) ? "" : id->type) > sizeof(buf)) {
+        if ((unsigned)snprintf(buf, sizeof(buf), "%s/%s/%s",
+                               (id->host == NULL) ? "" : id->host,
+                               (id->plugin == NULL) ? "" : id->plugin,
+                               (id->type == NULL) ? "" : id->type) >
+            sizeof(buf)) {
           error++;
           continue;
         }

diff --git a/src/utils/cmds/putmetric.c b/src/utils/cmds/putmetric.c
index d5727297a64c85a2052fe2dc73297a11f4348572..9316609a626ab1a797af56a0fd1d680818d2e8f3 100644 (file)
--- a/src/utils/cmds/putmetric.c
+++ b/src/utils/cmds/putmetric.c
@@ -37,7 +37,7 @@
 
 /* TODO(octo): add an option to set metric->value_type */
 static int set_option(metric_t *m, char const *key, char const *value,
-                      cmd_error_handler_t *err) {
+                      __attribute__((unused)) cmd_error_handler_t *err) {
   if ((m == NULL) || (key == NULL) || (value == NULL))
     return -1;
 

diff --git a/src/utils/cmds/putval.c b/src/utils/cmds/putval.c
index 76078676c089be4f98a183dc47c975176219e138..7a094ba590757ff146cedc6d0d6f8cc96cbd9b7f 100644 (file)
--- a/src/utils/cmds/putval.c
+++ b/src/utils/cmds/putval.c
@@ -36,7 +36,7 @@
  */
 
 static int set_option(value_list_t *vl, char const *key, char const *value,
-                      cmd_error_handler_t *errhndl) {
+                      __attribute__((unused)) cmd_error_handler_t *errhndl) {
   if ((vl == NULL) || (key == NULL) || (value == NULL)) {
     return EINVAL;
   }

diff --git a/src/write_prometheus.c b/src/write_prometheus.c
index 567b8690cf1ccc99830ec1b63610a052b020d804..cdb6cb705d3edd879cf2264704f745031df0b8b2 100644 (file)
--- a/src/write_prometheus.c
+++ b/src/write_prometheus.c
@@ -120,21 +120,23 @@ static void format_text(strbuf_t *buf) {
 
 /* http_handler is the callback called by the microhttpd library. It essentially
  * handles all HTTP request aspects and creates an HTTP response. */
-static MHD_RESULT http_handler(void *cls, struct MHD_Connection *connection,
-                               const char *url, const char *method,
-                               const char *version, const char *upload_data,
-                               size_t *upload_data_size,
+static MHD_RESULT http_handler(__attribute__((unused)) void *cls,
+                               struct MHD_Connection *connection,
+                               __attribute__((unused)) const char *url,
+                               const char *method,
+                               __attribute__((unused)) const char *version,
+                               __attribute__((unused)) const char *upload_data,
+                               __attribute__((unused)) size_t *upload_data_size,
                                void **connection_state) {
   if (strcmp(method, MHD_HTTP_METHOD_GET) != 0) {
     return MHD_NO;
   }
 
-  /* On the first call for each connection, return without anything further.
-   * Apparently not everything has been initialized yet or so; the docs are not
-   * very specific on the issue. */
+  /* According to documentation, first call for each connection is after headers
+   * have been parsed, and should be used only for reporting errors */
   if (*connection_state == NULL) {
-    /* set to a random non-NULL pointer. */
-    *connection_state = &(int){42};
+    /* keep track of connection state */
+    *connection_state = &"called";
     return MHD_YES;
   }
 
@@ -491,6 +493,7 @@ static int prom_missing(metric_family_t const *fam,
         continue;
       }
       metric_family_free(prom_fam);
+      break;
     }
   }
 

diff --git a/src/write_syslog.c b/src/write_syslog.c
index c5d75a1d24274eed1a807ba8d7f153cf6ca219f5..b14b121af51f13ea8ae2a9af5c83bfe62cc084e0 100644 (file)
--- a/src/write_syslog.c
+++ b/src/write_syslog.c
@@ -371,7 +371,7 @@ static int ws_format_values(char *ret, size_t ret_len, int ds_num,
 }
 
 static int ws_format_name(char *ret, int ret_len, const value_list_t *vl,
-                          const struct ws_callback *cb, const char *ds_name) {
+                          const char *ds_name) {
 
   if (ds_name != NULL) {
     snprintf(ret, ret_len, "%s.%s", vl->type, ds_name);
@@ -507,7 +507,7 @@ static int ws_write_messages(const data_set_t *ds, const value_list_t *vl,
       ds_name = ds->ds[i].name;
 
     /* Copy the identifier to 'key' and escape it. */
-    status = ws_format_name(key, sizeof(key), vl, cb, ds_name);
+    status = ws_format_name(key, sizeof(key), vl, ds_name);
     if (status != 0) {
       ERROR("write_syslog plugin: error with format_name");
       return status;

diff --git a/src/write_tsdb.c b/src/write_tsdb.c
index da887241b2e66078b7131f1d61131d2f48f250a0..a3a480d6ca50de583d811e9c0c72875a565fc62b 100644 (file)
--- a/src/write_tsdb.c
+++ b/src/write_tsdb.c
@@ -364,7 +364,7 @@ static int wt_format_values(char *ret, size_t ret_len, int ds_num,
 }
 
 static int wt_format_name(char *ret, int ret_len, const value_list_t *vl,
-                          const struct wt_callback *cb, const char *ds_name) {
+                          const char *ds_name) {
   int status;
   char *temp = NULL;
   const char *prefix = "";
@@ -525,7 +525,7 @@ static int wt_write_messages(const data_set_t *ds, const value_list_t *vl,
       ds_name = ds->ds[i].name;
 
     /* Copy the identifier to 'key' and escape it. */
-    status = wt_format_name(key, sizeof(key), vl, cb, ds_name);
+    status = wt_format_name(key, sizeof(key), vl, ds_name);
     if (status != 0) {
       ERROR("write_tsdb plugin: error with format_name");
       return status;