Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[collectd 6] Compiler warnings for collectd core with stricter compilation options #4241

Open
eero-t opened this issue Jan 18, 2024 · 3 comments

Comments

@eero-t
Copy link
Contributor

eero-t commented Jan 18, 2024

  • Version of collectd: collectd-6.0 git
  • Operating system / distribution: Fedora 38

Steps to reproduce

  • Prepare config.h: ./configure
  • build (most of) collectd core files with stricter warnings:
for i in $(find src/ -mindepth 3 -name '*.c' | grep -v -e /dpdk/ -e format_json.c -e format_kairosdb.c -e _mock.c -e _test.c -e _windows.c); do \
  echo "*** $i ***"; \
  gcc -I. -Isrc -Isrc/daemon -Isrc/libcollectdclient -DHAVE_CONFIG_H -O3 -Werror -Wall -Wextra -Wno-missing-field-initializers -Wcast-align=strict -Wformat-security -Wnull-dereference -Wstrict-overflow=2 -Warray-bounds=2 -D_FORTIFY_SOURCE=2 -Wno-aggressive-loop-optimizations -c -o /dev/null $i; \
done

(I do not have compatible include files for the files I've left out, and warnings from mock/test/windows files were not that relevant. Proper warning analysis requires use of high optimization level.)

Actual output

Output shows few warnings:

*** src/utils/cmds/listval.c ***
*** src/utils/cmds/putnotif.c ***
*** src/utils/cmds/putval.c ***
*** src/utils/cmds/putmetric.c ***
*** src/utils/cmds/flush.c ***
*** src/utils/cmds/parse_option.c ***
*** src/utils/cmds/getthreshold.c ***
*** src/utils/cmds/getval.c ***
*** src/utils/cmds/cmds.c ***
*** src/utils/curl_stats/curl_stats.c ***
*** src/utils/resource_metrics/resource_metrics.c ***
*** src/utils/value_list/value_list.c ***
*** src/utils/format_json/open_telemetry.c ***
*** src/utils/ignorelist/ignorelist.c ***
*** src/utils/crc32/crc32.c ***
*** src/utils/format_influxdb/format_influxdb.c ***
*** src/utils/gce/gce.c ***
*** src/utils/avltree/avltree.c ***
*** src/utils/oauth/oauth.c ***
*** src/utils/taskstats/taskstats.c ***
*** src/utils/utf8/utf8.c ***
*** src/utils/ovs/ovs.c ***
src/utils/ovs/ovs.c: In function ‘ovs_yajl_gen_val’:
src/utils/ovs/ovs.c:365:13: error: potential null pointer dereference [-Werror=null-dereference]
  365 |     obj_len = YAJL_GET_OBJECT(jval)->len;
      |     ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/utils/ovs/ovs.c:368:41: error: potential null pointer dereference [-Werror=null-dereference]
  368 |       jobj_value = YAJL_GET_OBJECT(jval)->values[i];
      |                                         ^
src/utils/ovs/ovs.c:367:38: error: null pointer dereference [-Werror=null-dereference]
  367 |       obj_key = YAJL_GET_OBJECT(jval)->keys[i];
      |                                      ^
cc1: all warnings being treated as errors
*** src/utils/heap/heap.c ***
*** src/utils/format_graphite/format_graphite.c ***
*** src/utils/rrdcreate/rrdcreate.c ***
*** src/utils/match/match.c ***
*** src/utils/tail/tail.c ***
*** src/utils/strbuf/strbuf.c ***
*** src/utils/lookup/vl_lookup.c ***
*** src/utils/message_parser/message_parser.c ***
src/utils/message_parser/message_parser.c: In function ‘start_message_assembly’:
src/utils/message_parser/message_parser.c:91:25: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
   91 |   if (self->message_idx >= self->messages_max_len) {
      |                         ^~
src/utils/message_parser/message_parser.c: In function ‘message_assembler’:
src/utils/message_parser/message_parser.c:150:42: error: unused parameter ‘row’ [-Werror=unused-parameter]
  150 | static int message_assembler(const char *row, char *const *matches,
      |                              ~~~~~~~~~~~~^~~
cc1: all warnings being treated as errors
*** src/utils/dmi/dmi.c ***
*** src/utils/dns/dns.c ***
src/utils/dns/dns.c: In function ‘ignore_list_add_name’:
src/utils/dns/dns.c:228:16: error: cast increases required alignment of target type [-Werror=cast-align]
  228 |              &((struct sockaddr_in *)ai_ptr->ai_addr)->sin_addr, 4);
      |                ^
src/utils/dns/dns.c:232:25: error: cast increases required alignment of target type [-Werror=cast-align]
  232 |       ignore_list_add(&((struct sockaddr_in6 *)ai_ptr->ai_addr)->sin6_addr);
      |                         ^
cc1: all warnings being treated as errors
*** src/utils/db_query/db_query.c ***
*** src/utils/format_stackdriver/format_stackdriver.c ***
src/utils/format_stackdriver/format_stackdriver.c: In function ‘read_cumulative_state’:
src/utils/format_stackdriver/format_stackdriver.c:295:36: error: comparison of integer expressions of different signedness: ‘int64_t’ {aka ‘long int’} and ‘counter_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
  295 |   bool is_reset = *ret_start_value > m->value.counter;
      |                                    ^
cc1: all warnings being treated as errors
*** src/utils/metadata/meta_data.c ***
*** src/utils/common/common.c ***
*** src/utils/latency/latency.c ***
*** src/utils/latency/latency_config.c ***
*** src/utils/proc_pids/proc_pids.c ***
src/utils/proc_pids/proc_pids.c: In function ‘pids_list_contains_pid’:
src/utils/proc_pids/proc_pids.c:119:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
  119 |   for (int i = 0; i < list->size; i++)
      |                     ^
src/utils/proc_pids/proc_pids.c: In function ‘pids_list_diff’:
src/utils/proc_pids/proc_pids.c:333:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
  333 |   for (int i = 0; i < proc->prev->size; i++)
      |                     ^
src/utils/proc_pids/proc_pids.c:340:21: error: comparison of integer expressions of different signedness: ‘int’ and ‘size_t’ {aka ‘long unsigned int’} [-Werror=sign-compare]
  340 |   for (int i = 0; i < proc->curr->size; i++)
      |                     ^
cc1: all warnings being treated as errors
*** src/utils/mount/mount.c ***
*** src/utils/config_cores/config_cores.c ***
@eero-t
Copy link
Contributor Author

eero-t commented Jan 18, 2024

If one uses -Wpendantic option:

$ for i in $(find src/ -mindepth 3 -name '*.c' | grep -v -e /dpdk/ -e format_json.c -e format_kairosdb.c -e _mock.c -e _test.c -e _windows.c); do \
  echo "*** $i ***"; \
  gcc -I. -Isrc -Isrc/daemon -Isrc/libcollectdclient -DHAVE_CONFIG_H -O3 -Werror -Wpedantic -c -o /dev/null $i; \
done

there are more warnings, about using void * (data pointers) when code requires function pointers:

*** src/utils/cmds/listval.c ***
*** src/utils/cmds/putnotif.c ***
*** src/utils/cmds/putval.c ***
*** src/utils/cmds/putmetric.c ***
*** src/utils/cmds/flush.c ***
*** src/utils/cmds/parse_option.c ***
*** src/utils/cmds/getthreshold.c ***
*** src/utils/cmds/getval.c ***
*** src/utils/cmds/cmds.c ***
*** src/utils/curl_stats/curl_stats.c ***
*** src/utils/resource_metrics/resource_metrics.c ***
src/utils/resource_metrics/resource_metrics.c: In function ‘lookup_resource’:
src/utils/resource_metrics/resource_metrics.c:43:18: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
   43 |                  (void *)resource_metrics_compare);
      |                  ^
src/utils/resource_metrics/resource_metrics.c:43:18: error: ISO C forbids passing argument 5 of ‘bsearch’ between function pointer and ‘void *’ [-Werror=pedantic]
   43 |                  (void *)resource_metrics_compare);
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/stdlib.h:846,
                 from src/daemon/collectd.h:50,
                 from src/daemon/plugin.h:32,
                 from src/utils/resource_metrics/resource_metrics.c:27:
/usr/include/bits/stdlib-bsearch.h:21:24: note: expected ‘__compar_fn_t’ {aka ‘int (*)(const void *, const void *)’} but argument is of type ‘void *’
   21 |          __compar_fn_t __compar)
      |          ~~~~~~~~~~~~~~^~~~~~~~
src/utils/resource_metrics/resource_metrics.c: In function ‘insert_resource’:
src/utils/resource_metrics/resource_metrics.c:64:9: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
   64 |         (void *)resource_metrics_compare);
      |         ^
src/utils/resource_metrics/resource_metrics.c:64:9: error: ISO C forbids passing argument 4 of ‘qsort’ between function pointer and ‘void *’ [-Werror=pedantic]
   64 |         (void *)resource_metrics_compare);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/stdlib.h:852:34: note: expected ‘__compar_fn_t’ {aka ‘int (*)(const void *, const void *)’} but argument is of type ‘void *’
  852 |                    __compar_fn_t __compar) __nonnull ((1, 4));
      |                    ~~~~~~~~~~~~~~^~~~~~~~
src/utils/resource_metrics/resource_metrics.c: In function ‘lookup_family’:
src/utils/resource_metrics/resource_metrics.c:94:15: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
   94 |               (void *)compare_family_by_name);
      |               ^
src/utils/resource_metrics/resource_metrics.c:94:15: error: ISO C forbids passing argument 5 of ‘bsearch’ between function pointer and ‘void *’ [-Werror=pedantic]
   94 |               (void *)compare_family_by_name);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/stdlib-bsearch.h:21:24: note: expected ‘__compar_fn_t’ {aka ‘int (*)(const void *, const void *)’} but argument is of type ‘void *’
   21 |          __compar_fn_t __compar)
      |          ~~~~~~~~~~~~~~^~~~~~~~
src/utils/resource_metrics/resource_metrics.c: In function ‘insert_family’:
src/utils/resource_metrics/resource_metrics.c:125:9: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
  125 |         (void *)compare_family_by_name);
      |         ^
src/utils/resource_metrics/resource_metrics.c:125:9: error: ISO C forbids passing argument 4 of ‘qsort’ between function pointer and ‘void *’ [-Werror=pedantic]
  125 |         (void *)compare_family_by_name);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/stdlib.h:852:34: note: expected ‘__compar_fn_t’ {aka ‘int (*)(const void *, const void *)’} but argument is of type ‘void *’
  852 |                    __compar_fn_t __compar) __nonnull ((1, 4));
      |                    ~~~~~~~~~~~~~~^~~~~~~~
src/utils/resource_metrics/resource_metrics.c: In function ‘metric_exists’:
src/utils/resource_metrics/resource_metrics.c:164:55: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
  164 |                             sizeof(*fam->metric.ptr), (void *)compare_metrics);
      |                                                       ^
src/utils/resource_metrics/resource_metrics.c:164:55: error: ISO C forbids passing argument 5 of ‘bsearch’ between function pointer and ‘void *’ [-Werror=pedantic]
  164 |                             sizeof(*fam->metric.ptr), (void *)compare_metrics);
      |                                                       ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bits/stdlib-bsearch.h:21:24: note: expected ‘__compar_fn_t’ {aka ‘int (*)(const void *, const void *)’} but argument is of type ‘void *’
   21 |          __compar_fn_t __compar)
      |          ~~~~~~~~~~~~~~^~~~~~~~
src/utils/resource_metrics/resource_metrics.c: In function ‘insert_metrics’:
src/utils/resource_metrics/resource_metrics.c:194:11: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
  194 |           (void *)compare_metrics);
      |           ^
src/utils/resource_metrics/resource_metrics.c:194:11: error: ISO C forbids passing argument 4 of ‘qsort’ between function pointer and ‘void *’ [-Werror=pedantic]
  194 |           (void *)compare_metrics);
      |           ^~~~~~~~~~~~~~~~~~~~~~~
/usr/include/stdlib.h:852:34: note: expected ‘__compar_fn_t’ {aka ‘int (*)(const void *, const void *)’} but argument is of type ‘void *’
  852 |                    __compar_fn_t __compar) __nonnull ((1, 4));
      |                    ~~~~~~~~~~~~~~^~~~~~~~
cc1: all warnings being treated as errors
*** src/utils/value_list/value_list.c ***
*** src/utils/format_json/open_telemetry.c ***
*** src/utils/ignorelist/ignorelist.c ***
*** src/utils/crc32/crc32.c ***
*** src/utils/format_influxdb/format_influxdb.c ***
*** src/utils/gce/gce.c ***
*** src/utils/avltree/avltree.c ***
*** src/utils/oauth/oauth.c ***
*** src/utils/taskstats/taskstats.c ***
*** src/utils/utf8/utf8.c ***
*** src/utils/ovs/ovs.c ***
*** src/utils/heap/heap.c ***
*** src/utils/format_graphite/format_graphite.c ***
*** src/utils/rrdcreate/rrdcreate.c ***
*** src/utils/match/match.c ***
*** src/utils/tail/tail.c ***
*** src/utils/strbuf/strbuf.c ***
*** src/utils/lookup/vl_lookup.c ***
*** src/utils/message_parser/message_parser.c ***
*** src/utils/dmi/dmi.c ***
*** src/utils/dns/dns.c ***
*** src/utils/db_query/db_query.c ***
*** src/utils/format_stackdriver/format_stackdriver.c ***
src/utils/format_stackdriver/format_stackdriver.c: In function ‘sd_output_create’:
src/utils/format_stackdriver/format_stackdriver.c:495:30: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
  495 |   out->staged = c_avl_create((void *)strcmp);
      |                              ^
src/utils/format_stackdriver/format_stackdriver.c:495:30: error: ISO C forbids passing argument 1 of ‘c_avl_create’ between function pointer and ‘void *’ [-Werror=pedantic]
  495 |   out->staged = c_avl_create((void *)strcmp);
      |                              ^~~~~~~~~~~~~~
In file included from src/utils/format_stackdriver/format_stackdriver.c:28:
src/utils/avltree/avltree.h:54:34: note: expected ‘int (*)(const void *, const void *)’ but argument is of type ‘void *’
   54 | c_avl_tree_t *c_avl_create(int (*compare)(const void *, const void *));
      |                            ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/utils/format_stackdriver/format_stackdriver.c:501:42: error: ISO C forbids conversion of function pointer to object pointer type [-Werror=pedantic]
  501 |   out->metric_descriptors = c_avl_create((void *)strcmp);
      |                                          ^
src/utils/format_stackdriver/format_stackdriver.c:501:42: error: ISO C forbids passing argument 1 of ‘c_avl_create’ between function pointer and ‘void *’ [-Werror=pedantic]
  501 |   out->metric_descriptors = c_avl_create((void *)strcmp);
      |                                          ^~~~~~~~~~~~~~
src/utils/avltree/avltree.h:54:34: note: expected ‘int (*)(const void *, const void *)’ but argument is of type ‘void *’
   54 | c_avl_tree_t *c_avl_create(int (*compare)(const void *, const void *));
      |                            ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
*** src/utils/metadata/meta_data.c ***
*** src/utils/common/common.c ***
*** src/utils/latency/latency.c ***
*** src/utils/latency/latency_config.c ***
*** src/utils/proc_pids/proc_pids.c ***
*** src/utils/mount/mount.c ***
*** src/utils/config_cores/config_cores.c ***

@eero-t
Copy link
Contributor Author

eero-t commented Jan 19, 2024

Above -Wpedantic output is based on current #4243. I'll add fixes to those cast warnings to it later.

@eero-t
Copy link
Contributor Author

eero-t commented Jan 30, 2024

Additional fixes for warnings listed here are in #4262 and #4264.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants