From 95a54dd0f69c04f67a256a5be57659c2e754fc12 Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 19 Jan 2024 12:35:14 +0200 Subject: [PATCH 1/9] daemon/utils_cache.c: fix -Wpedantic warnings Semicolons are nice for editor indenting, but redundant ones are against ISO C. This can be fixed by adding do/while(0) loop to macro, which requires also making functions otherwise look like normal functions. Signed-off-by: Eero Tamminen --- src/daemon/utils_cache.c | 82 +++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 39 deletions(-) diff --git a/src/daemon/utils_cache.c b/src/daemon/utils_cache.c index 9702318835..5c7a6af3c7 100644 --- a/src/daemon/utils_cache.c +++ b/src/daemon/utils_cache.c @@ -909,7 +909,7 @@ static meta_data_t *uc_get_meta(metric_t const *m) /* {{{ */ /* Sorry about this preprocessor magic, but it really makes this file much * shorter.. */ #define UC_WRAP(wrap_function) \ - { \ + do { \ pthread_mutex_lock(&cache_lock); \ errno = 0; \ meta_data_t *meta = uc_get_meta(m); \ @@ -920,23 +920,23 @@ static meta_data_t *uc_get_meta(metric_t const *m) /* {{{ */ int ret = wrap_function(meta, key); \ pthread_mutex_unlock(&cache_lock); \ return ret; \ - } - -int uc_meta_data_exists(metric_t const *m, const char *key) - UC_WRAP(meta_data_exists); - -int uc_meta_data_delete(metric_t const *m, const char *key) - UC_WRAP(meta_data_delete); + } while (0) +int uc_meta_data_exists(metric_t const *m, const char *key) { + UC_WRAP(meta_data_exists); +} +int uc_meta_data_delete(metric_t const *m, const char *key) { + UC_WRAP(meta_data_delete); +} /* The second argument is called `toc` in the API, but the macro expects * `key`. */ -int uc_meta_data_toc(metric_t const *m, char ***key) UC_WRAP(meta_data_toc); +int uc_meta_data_toc(metric_t const *m, char ***key) { UC_WRAP(meta_data_toc); } #undef UC_WRAP /* We need a new version of this macro because the following functions take - * two argumetns. gratituous semicolons added for formatting sanity*/ + * two arguments. Gratuitous semicolons added for formatting sanity. */ #define UC_WRAP(wrap_function) \ - { \ + do { \ pthread_mutex_lock(&cache_lock); \ errno = 0; \ meta_data_t *meta = uc_get_meta(m); \ @@ -947,37 +947,41 @@ int uc_meta_data_toc(metric_t const *m, char ***key) UC_WRAP(meta_data_toc); int ret = wrap_function(meta, key, value); \ pthread_mutex_unlock(&cache_lock); \ return ret; \ - } -int uc_meta_data_add_string(metric_t const *m, const char *key, - const char *value) UC_WRAP(meta_data_add_string); + } while (0) +int uc_meta_data_add_string(metric_t const *m, const char *key, + const char *value) { + UC_WRAP(meta_data_add_string); +} int uc_meta_data_add_signed_int(metric_t const *m, const char *key, - int64_t value) - UC_WRAP(meta_data_add_signed_int); - + int64_t value) { + UC_WRAP(meta_data_add_signed_int); +} int uc_meta_data_add_unsigned_int(metric_t const *m, const char *key, - uint64_t value) - UC_WRAP(meta_data_add_unsigned_int); - -int uc_meta_data_add_double(metric_t const *m, const char *key, double value) - UC_WRAP(meta_data_add_double); -int uc_meta_data_add_boolean(metric_t const *m, const char *key, bool value) - UC_WRAP(meta_data_add_boolean); - -int uc_meta_data_get_string(metric_t const *m, const char *key, char **value) - UC_WRAP(meta_data_get_string); - + uint64_t value) { + UC_WRAP(meta_data_add_unsigned_int); +} +int uc_meta_data_add_double(metric_t const *m, const char *key, double value) { + UC_WRAP(meta_data_add_double); +} +int uc_meta_data_add_boolean(metric_t const *m, const char *key, bool value) { + UC_WRAP(meta_data_add_boolean); +} +int uc_meta_data_get_string(metric_t const *m, const char *key, char **value) { + UC_WRAP(meta_data_get_string); +} int uc_meta_data_get_signed_int(metric_t const *m, const char *key, - int64_t *value) - UC_WRAP(meta_data_get_signed_int); - + int64_t *value) { + UC_WRAP(meta_data_get_signed_int); +} int uc_meta_data_get_unsigned_int(metric_t const *m, const char *key, - uint64_t *value) - UC_WRAP(meta_data_get_unsigned_int); - -int uc_meta_data_get_double(metric_t const *m, const char *key, double *value) - UC_WRAP(meta_data_get_double); - -int uc_meta_data_get_boolean(metric_t const *m, const char *key, bool *value) - UC_WRAP(meta_data_get_boolean); + uint64_t *value) { + UC_WRAP(meta_data_get_unsigned_int); +} +int uc_meta_data_get_double(metric_t const *m, const char *key, double *value) { + UC_WRAP(meta_data_get_double); +} +int uc_meta_data_get_boolean(metric_t const *m, const char *key, bool *value) { + UC_WRAP(meta_data_get_boolean); +} #undef UC_WRAP From 0215fa20c6bcb5a8e538f63915f153a6d86f748e Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 19 Jan 2024 18:09:09 +0200 Subject: [PATCH 2/9] Pass strcmp() to c_avl_create() as function pointer, not data one Matches how strcmp() is passed to c_avl_create() elsewhere. Reported by -Wpendantic. Signed-off-by: Eero Tamminen --- src/redfish.c | 2 +- src/utils/format_stackdriver/format_stackdriver.c | 5 +++-- src/write_prometheus.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/redfish.c b/src/redfish.c index d2d7748207..c3cb85aa9a 100644 --- a/src/redfish.c +++ b/src/redfish.c @@ -275,7 +275,7 @@ static int redfish_preconfig(void) { goto error; /* Creating placeholder for queries */ - ctx.queries = c_avl_create((void *)strcmp); + ctx.queries = c_avl_create((int (*)(const void *, const void *))strcmp); if (ctx.services == NULL) goto free_services; diff --git a/src/utils/format_stackdriver/format_stackdriver.c b/src/utils/format_stackdriver/format_stackdriver.c index 0900361b3b..6e2e4e2262 100644 --- a/src/utils/format_stackdriver/format_stackdriver.c +++ b/src/utils/format_stackdriver/format_stackdriver.c @@ -492,13 +492,14 @@ sd_output_t *sd_output_create(sd_resource_t *res) /* {{{ */ return NULL; } - out->staged = c_avl_create((void *)strcmp); + out->staged = c_avl_create((int (*)(const void *, const void *))strcmp); if (out->staged == NULL) { sd_output_destroy(out); return NULL; } - out->metric_descriptors = c_avl_create((void *)strcmp); + out->metric_descriptors = + c_avl_create((int (*)(const void *, const void *))strcmp); if (out->metric_descriptors == NULL) { sd_output_destroy(out); return NULL; diff --git a/src/write_prometheus.c b/src/write_prometheus.c index fd663c4d0e..430cf3b726 100644 --- a/src/write_prometheus.c +++ b/src/write_prometheus.c @@ -732,7 +732,7 @@ static int prom_config(oconfig_item_t *ci) { static int prom_init() { if (prom_metrics == NULL) { - prom_metrics = c_avl_create((void *)strcmp); + prom_metrics = c_avl_create((int (*)(const void *, const void *))strcmp); if (prom_metrics == NULL) { ERROR("write_prometheus plugin: c_avl_create() failed."); return -1; From 359058389756e9e46707b4f9ead3147f80bb5c1f Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 19 Jan 2024 18:31:54 +0200 Subject: [PATCH 3/9] resource_metrics.c: fix -Wpedantic warnings for casts, option 1 By casting function types to ones expected by bsearch() & qsort(). Signed-off-by: Eero Tamminen --- src/utils/resource_metrics/resource_metrics.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/utils/resource_metrics/resource_metrics.c b/src/utils/resource_metrics/resource_metrics.c index 1182445a65..371dc6c109 100644 --- a/src/utils/resource_metrics/resource_metrics.c +++ b/src/utils/resource_metrics/resource_metrics.c @@ -28,6 +28,8 @@ #include "utils/common/common.h" #include "utils/resource_metrics/resource_metrics.h" +typedef int (*compare_fn_t)(const void *, const void *); + static int resource_metrics_compare(resource_metrics_t const *a, resource_metrics_t const *b) { return label_set_compare(a->resource, b->resource); @@ -40,7 +42,7 @@ static resource_metrics_t *lookup_resource(resource_metrics_set_t *set, }; return bsearch(&key, set->ptr, set->num, sizeof(*set->ptr), - (void *)resource_metrics_compare); + (compare_fn_t)resource_metrics_compare); } static int insert_resource(resource_metrics_set_t *set, label_set_t resource) { @@ -61,7 +63,7 @@ static int insert_resource(resource_metrics_set_t *set, label_set_t resource) { set->num++; qsort(set->ptr, set->num, sizeof(*set->ptr), - (void *)resource_metrics_compare); + (compare_fn_t)resource_metrics_compare); return 0; } @@ -91,7 +93,7 @@ static metric_family_t *lookup_family(resource_metrics_t *rm, metric_family_t const *fam) { metric_family_t **ret = bsearch(&fam, rm->families, rm->families_num, sizeof(*rm->families), - (void *)compare_family_by_name); + (compare_fn_t)compare_family_by_name); if (ret == NULL) { return NULL; } @@ -122,7 +124,7 @@ static int insert_family(resource_metrics_t *rm, metric_family_t const *fam) { rm->families_num++; qsort(rm->families, rm->families_num, sizeof(*rm->families), - (void *)compare_family_by_name); + (compare_fn_t)compare_family_by_name); return 0; } @@ -160,8 +162,9 @@ static int compare_metrics(metric_t const *a, metric_t const *b) { } static bool metric_exists(metric_family_t const *fam, metric_t const *m) { - metric_t *found = bsearch(m, fam->metric.ptr, fam->metric.num, - sizeof(*fam->metric.ptr), (void *)compare_metrics); + metric_t *found = + bsearch(m, fam->metric.ptr, fam->metric.num, sizeof(*fam->metric.ptr), + (compare_fn_t)compare_metrics); return found != NULL; } @@ -191,7 +194,7 @@ static int insert_metrics(metric_family_t *fam, metric_list_t metrics) { if (((size_t)skipped) != metrics.num) { qsort(fam->metric.ptr, fam->metric.num, sizeof(*fam->metric.ptr), - (void *)compare_metrics); + (compare_fn_t)compare_metrics); } return skipped; From 98f04c2d7fd9f7ced827edb928d6dbb4a7373e45 Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 26 Jan 2024 18:49:06 +0200 Subject: [PATCH 4/9] resource_metrics.c: fix -Wpedantic warnings for casts, option 2 By changing compare function signatures to ones expected by bsearch() & qsort(). Signed-off-by: Eero Tamminen --- src/utils/resource_metrics/resource_metrics.c | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/utils/resource_metrics/resource_metrics.c b/src/utils/resource_metrics/resource_metrics.c index 371dc6c109..9d41eae81c 100644 --- a/src/utils/resource_metrics/resource_metrics.c +++ b/src/utils/resource_metrics/resource_metrics.c @@ -28,10 +28,9 @@ #include "utils/common/common.h" #include "utils/resource_metrics/resource_metrics.h" -typedef int (*compare_fn_t)(const void *, const void *); - -static int resource_metrics_compare(resource_metrics_t const *a, - resource_metrics_t const *b) { +static int resource_metrics_compare(void const *ra, void const *rb) { + resource_metrics_t const *a = (resource_metrics_t const *)ra; + resource_metrics_t const *b = (resource_metrics_t const *)rb; return label_set_compare(a->resource, b->resource); } @@ -42,7 +41,7 @@ static resource_metrics_t *lookup_resource(resource_metrics_set_t *set, }; return bsearch(&key, set->ptr, set->num, sizeof(*set->ptr), - (compare_fn_t)resource_metrics_compare); + resource_metrics_compare); } static int insert_resource(resource_metrics_set_t *set, label_set_t resource) { @@ -62,8 +61,7 @@ static int insert_resource(resource_metrics_set_t *set, label_set_t resource) { } set->num++; - qsort(set->ptr, set->num, sizeof(*set->ptr), - (compare_fn_t)resource_metrics_compare); + qsort(set->ptr, set->num, sizeof(*set->ptr), resource_metrics_compare); return 0; } @@ -85,7 +83,9 @@ lookup_or_insert_resource(resource_metrics_set_t *set, label_set_t resource) { return ret; } -static int compare_family_by_name(metric_family_t **a, metric_family_t **b) { +static int compare_family_by_name(void const *ma, void const *mb) { + metric_family_t **a = (metric_family_t **)ma; + metric_family_t **b = (metric_family_t **)mb; return strcmp((*a)->name, (*b)->name); } @@ -93,7 +93,7 @@ static metric_family_t *lookup_family(resource_metrics_t *rm, metric_family_t const *fam) { metric_family_t **ret = bsearch(&fam, rm->families, rm->families_num, sizeof(*rm->families), - (compare_fn_t)compare_family_by_name); + compare_family_by_name); if (ret == NULL) { return NULL; } @@ -124,7 +124,7 @@ static int insert_family(resource_metrics_t *rm, metric_family_t const *fam) { rm->families_num++; qsort(rm->families, rm->families_num, sizeof(*rm->families), - (compare_fn_t)compare_family_by_name); + compare_family_by_name); return 0; } @@ -146,7 +146,9 @@ static metric_family_t *lookup_or_insert_family(resource_metrics_t *rm, return ret; } -static int compare_metrics(metric_t const *a, metric_t const *b) { +static int compare_metrics(void const *ma, void const *mb) { + metric_t const *a = (metric_t const *)ma; + metric_t const *b = (metric_t const *)mb; int cmp = label_set_compare(a->label, b->label); if (cmp != 0) { return cmp; @@ -162,9 +164,8 @@ static int compare_metrics(metric_t const *a, metric_t const *b) { } static bool metric_exists(metric_family_t const *fam, metric_t const *m) { - metric_t *found = - bsearch(m, fam->metric.ptr, fam->metric.num, sizeof(*fam->metric.ptr), - (compare_fn_t)compare_metrics); + metric_t *found = bsearch(m, fam->metric.ptr, fam->metric.num, + sizeof(*fam->metric.ptr), compare_metrics); return found != NULL; } @@ -194,7 +195,7 @@ static int insert_metrics(metric_family_t *fam, metric_list_t metrics) { if (((size_t)skipped) != metrics.num) { qsort(fam->metric.ptr, fam->metric.num, sizeof(*fam->metric.ptr), - (compare_fn_t)compare_metrics); + compare_metrics); } return skipped; From 64a10f8b5944bc044cc5d9786d1eb3eca9bb558c Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Tue, 23 Jan 2024 11:51:42 +0200 Subject: [PATCH 5/9] .gitignore: simplify + add more things to ignore Signed-off-by: Eero Tamminen --- .gitignore | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/.gitignore b/.gitignore index afdbe5cd03..3cd7eb291d 100644 --- a/.gitignore +++ b/.gitignore @@ -13,13 +13,7 @@ Makefile.in /install-sh /libltdl/ /ltmain.sh -/m4/libtool.m4 -/m4/ltargz.m4 -/m4/ltdl.m4 -/m4/lt~obsolete.m4 -/m4/ltoptions.m4 -/m4/ltsugar.m4 -/m4/ltversion.m4 +/m4/ /missing /src/config.h.in @@ -40,6 +34,7 @@ src/stamp-h1 .dirstamp .libs/ .deps/ +/collectd /collectd-nagios /collectd-tg /collectdctl @@ -87,12 +82,13 @@ bindings/perl/pm_to_blib *.pyc # backup stuff +*.bak *~ # lint stuff *.ln -#ide stuff +# IDE stuff .vscode # cscope stuff @@ -103,5 +99,6 @@ test-suite.log src/tests/ test_* -# src/daemon/... -/collectd +# coverage data +*.gcov +*.gcno From 45702ca769e66e22955878d35fb15b919e60f869 Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 26 Jan 2024 18:21:33 +0200 Subject: [PATCH 6/9] utf8_test.c: fix -Wpedantic sign issue While GCC defaults to char being unsigned, tests would fail when char is signed, so just declare strings to be uint8_t for utf8. Signed-off-by: Eero Tamminen --- src/utils/utf8/utf8_test.c | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/utils/utf8/utf8_test.c b/src/utils/utf8/utf8_test.c index c90705c0f8..9a147cafe8 100644 --- a/src/utils/utf8/utf8_test.c +++ b/src/utils/utf8/utf8_test.c @@ -28,17 +28,17 @@ DEF_TEST(utf8_valid) { struct { char const *name; - char *input; + uint8_t *input; bool want; } cases[] = { { .name = "simple string", - .input = "Hello, World!", + .input = (uint8_t *)"Hello, World!", .want = true, }, { .name = "empty string", - .input = "", + .input = (uint8_t *)"", .want = true, }, { @@ -48,56 +48,56 @@ DEF_TEST(utf8_valid) { }, { .name = "The greek work \"kosme\"", - .input = (char[]){0xce, 0xba, 0xe1, 0xbd, 0xb9, 0xcf, 0x83, 0xce, - 0xbc, 0xce, 0xb5, 0}, + .input = (uint8_t[]){0xce, 0xba, 0xe1, 0xbd, 0xb9, 0xcf, 0x83, 0xce, + 0xbc, 0xce, 0xb5, 0}, .want = true, }, { .name = "First possible sequence of three bytes", - .input = (char[]){0xe0, 0xa0, 0x80, 0}, + .input = (uint8_t[]){0xe0, 0xa0, 0x80, 0}, .want = true, }, { .name = "First possible sequence of four bytes", - .input = (char[]){0xf0, 0x90, 0x80, 0x80, 0}, + .input = (uint8_t[]){0xf0, 0x90, 0x80, 0x80, 0}, .want = true, }, { .name = "U-0000D7F", - .input = (char[]){0xed, 0x9f, 0xbf, 0}, + .input = (uint8_t[]){0xed, 0x9f, 0xbf, 0}, .want = true, }, { .name = "0xFE (invalid byte)", - .input = (char[]){'H', 0xfe, 'l', 'l', 'o', 0}, + .input = (uint8_t[]){'H', 0xfe, 'l', 'l', 'o', 0}, .want = false, }, { .name = "0xFF (invalid byte)", - .input = (char[]){'C', 'o', 0xff, 'e', 'e', 0}, + .input = (uint8_t[]){'C', 'o', 0xff, 'e', 'e', 0}, .want = false, }, { .name = "Continuation byte at end of string", - .input = (char[]){0xce, 0xba, 0xe1, 0xbd, 0xb9, 0xcf, 0x83, 0xce, - 0xbc, 0xce, 0}, + .input = (uint8_t[]){0xce, 0xba, 0xe1, 0xbd, 0xb9, 0xcf, 0x83, 0xce, + 0xbc, 0xce, 0}, .want = false, }, { .name = "U+002F (overlong ASCII character, 2 bytes)", - .input = (char[]){0xc0, 0xaf, 0}, + .input = (uint8_t[]){0xc0, 0xaf, 0}, .want = false, }, { .name = "U+002F (overlong ASCII character, 3 bytes)", - .input = (char[]){0xe0, 0x80, 0xaf, 0}, + .input = (uint8_t[]){0xe0, 0x80, 0xaf, 0}, .want = false, }, }; for (size_t i = 0; i < STATIC_ARRAY_SIZE(cases); i++) { printf("Case #%zu: %s\n", i, cases[i].name); - EXPECT_EQ_INT(cases[i].want, utf8_valid(cases[i].input)); + EXPECT_EQ_INT(cases[i].want, utf8_valid((const char *)cases[i].input)); } return 0; } From 6b24fb6b2ab4b8c75877d4d06afc5b024079d347 Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 26 Jan 2024 18:20:41 +0200 Subject: [PATCH 7/9] value_list_test.c: add missing include For STATIC_ARRAY_SIZE. Signed-off-by: Eero Tamminen --- src/utils/value_list/value_list_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/utils/value_list/value_list_test.c b/src/utils/value_list/value_list_test.c index 07b8856832..bbfdcf0a9b 100644 --- a/src/utils/value_list/value_list_test.c +++ b/src/utils/value_list/value_list_test.c @@ -25,6 +25,7 @@ */ #include "testing.h" +#include "utils/common/common.h" #include "utils/value_list/value_list.h" DEF_TEST(parse_values) { From b3f02f4bdecebfd9f38f2f65d2cd4158beb48258 Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 26 Jan 2024 19:02:04 +0200 Subject: [PATCH 8/9] vl_lookup_test.c: -Wpedantic lookup_create() arg cast fixes Signed-off-by: Eero Tamminen --- src/utils/lookup/vl_lookup_test.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/utils/lookup/vl_lookup_test.c b/src/utils/lookup/vl_lookup_test.c index 2dd53ce3fc..0c9839ba22 100644 --- a/src/utils/lookup/vl_lookup_test.c +++ b/src/utils/lookup/vl_lookup_test.c @@ -129,7 +129,8 @@ static int checked_lookup_search(lookup_t *obj, char const *host, DEF_TEST(group_by_specific_host) { lookup_t *obj; CHECK_NOT_NULL(obj = lookup_create(lookup_class_callback, lookup_obj_callback, - (void *)free, (void *)free)); + (lookup_free_class_callback_t)free, + (lookup_free_obj_callback_t)free)); checked_lookup_add(obj, "/.*/", "test", "", "test", "/.*/", LU_GROUP_BY_HOST); checked_lookup_search(obj, "host0", "test", "", "test", "0", @@ -148,7 +149,8 @@ DEF_TEST(group_by_specific_host) { DEF_TEST(group_by_any_host) { lookup_t *obj; CHECK_NOT_NULL(obj = lookup_create(lookup_class_callback, lookup_obj_callback, - (void *)free, (void *)free)); + (lookup_free_class_callback_t)free, + (lookup_free_obj_callback_t)free)); checked_lookup_add(obj, "/.*/", "/.*/", "/.*/", "test", "/.*/", LU_GROUP_BY_HOST); @@ -178,7 +180,8 @@ DEF_TEST(multiple_lookups) { int status; CHECK_NOT_NULL(obj = lookup_create(lookup_class_callback, lookup_obj_callback, - (void *)free, (void *)free)); + (lookup_free_class_callback_t)free, + (lookup_free_obj_callback_t)free)); checked_lookup_add(obj, "/.*/", "plugin0", "", "test", "/.*/", LU_GROUP_BY_HOST); @@ -204,7 +207,8 @@ DEF_TEST(multiple_lookups) { DEF_TEST(regex) { lookup_t *obj; CHECK_NOT_NULL(obj = lookup_create(lookup_class_callback, lookup_obj_callback, - (void *)free, (void *)free)); + (lookup_free_class_callback_t)free, + (lookup_free_obj_callback_t)free)); checked_lookup_add(obj, "/^db[0-9]\\./", "cpu", "/.*/", "cpu", "/.*/", LU_GROUP_BY_TYPE_INSTANCE); From d9d5025a064584380a5e4dacfe2c56220a06668d Mon Sep 17 00:00:00 2001 From: Eero Tamminen Date: Fri, 26 Jan 2024 19:51:20 +0200 Subject: [PATCH 9/9] plugin.c: -Wpendantic cast fixes for callback handling Remove void* _data_ pointer casts with correct function pointer types and casts. In some cases this allowed dropping the cast completely, as I used same type for "cf_callback" member as the simplest init() and shutdown() callbacks. Only remaining pedantic warning is unfixable, as dlsym() API returns void* data pointer, instead of function pointer. Signed-off-by: Eero Tamminen --- src/daemon/plugin.c | 67 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 32 deletions(-) diff --git a/src/daemon/plugin.c b/src/daemon/plugin.c index 72ff571375..45155cb060 100644 --- a/src/daemon/plugin.c +++ b/src/daemon/plugin.c @@ -60,11 +60,14 @@ #include +/* except for plugin_log_cb, all callback types return an int */ +typedef int (*placeholder_cb_t)(void); + /* * Private structures */ struct callback_func_s { - void *cf_callback; + placeholder_cb_t cf_callback; user_data_t cf_udata; plugin_ctx_t cf_ctx; }; @@ -349,7 +352,7 @@ static int register_callback(llist_t **list, /* {{{ */ } /* }}} int register_callback */ static int create_register_callback(llist_t **list, /* {{{ */ - const char *name, void *callback, + const char *name, placeholder_cb_t callback, user_data_t const *ud) { if (name == NULL || callback == NULL) @@ -427,7 +430,7 @@ static int plugin_load_file(char const *file, bool global) { return ENOENT; } - void (*reg_handle)(void) = (void *)dlsym(dlh, "module_register"); + void (*reg_handle)(void) = dlsym(dlh, "module_register"); if (reg_handle == NULL) { ERROR("Couldn't find symbol \"module_register\" in \"%s\": %s\n", file, dlerror()); @@ -519,13 +522,13 @@ static void *plugin_read_thread(void __attribute__((unused)) * args) { old_ctx = plugin_set_ctx(rf->rf_ctx); if (rf_type == RF_SIMPLE) { - int (*callback)(void) = (void *)rf->rf_callback; + int (*callback)(void) = rf->rf_callback; status = (*callback)(); } else { assert(rf_type == RF_COMPLEX); - plugin_read_cb callback = (void *)rf->rf_callback; + plugin_read_cb callback = (plugin_read_cb)rf->rf_callback; status = (*callback)(&rf->rf_udata); } @@ -1055,8 +1058,8 @@ EXPORT int plugin_register_complex_config(const char *type, return cf_register_complex(type, callback); } /* int plugin_register_complex_config */ -EXPORT int plugin_register_init(const char *name, int (*callback)(void)) { - return create_register_callback(&list_init, name, (void *)callback, NULL); +EXPORT int plugin_register_init(const char *name, plugin_init_cb callback) { + return create_register_callback(&list_init, name, callback, NULL); } /* plugin_register_init */ static int plugin_compare_read_func(const void *arg0, const void *arg1) { @@ -1147,7 +1150,7 @@ EXPORT int plugin_register_read(const char *name, int (*callback)(void)) { return ENOMEM; } - rf->rf_callback = (void *)callback; + rf->rf_callback = callback; rf->rf_udata.data = NULL; rf->rf_udata.free_func = NULL; rf->rf_ctx = plugin_get_ctx(); @@ -1180,7 +1183,7 @@ EXPORT int plugin_register_complex_read(const char *group, const char *name, return ENOMEM; } - rf->rf_callback = (void *)callback; + rf->rf_callback = (placeholder_cb_t)callback; if (group != NULL) sstrncpy(rf->rf_group, group, sizeof(rf->rf_group)); else @@ -1299,8 +1302,8 @@ EXPORT int plugin_register_flush(const char *name, plugin_flush_cb callback, user_data_t const *ud) { plugin_ctx_t ctx = plugin_get_ctx(); - int status = - create_register_callback(&list_flush, name, (void *)callback, ud); + int status = create_register_callback(&list_flush, name, + (placeholder_cb_t)callback, ud); if (status != 0) { return status; } @@ -1347,7 +1350,8 @@ EXPORT int plugin_register_flush(const char *name, plugin_flush_cb callback, EXPORT int plugin_register_missing(const char *name, plugin_missing_cb callback, user_data_t const *ud) { - return create_register_callback(&list_missing, name, (void *)callback, ud); + return create_register_callback(&list_missing, name, + (placeholder_cb_t)callback, ud); } /* int plugin_register_missing */ EXPORT int plugin_register_cache_event(const char *name, @@ -1405,20 +1409,22 @@ EXPORT int plugin_register_cache_event(const char *name, return 0; } /* int plugin_register_cache_event */ -EXPORT int plugin_register_shutdown(const char *name, int (*callback)(void)) { - return create_register_callback(&list_shutdown, name, (void *)callback, NULL); +EXPORT int plugin_register_shutdown(const char *name, + plugin_shutdown_cb callback) { + return create_register_callback(&list_shutdown, name, callback, NULL); } /* int plugin_register_shutdown */ EXPORT int plugin_register_log(const char *name, plugin_log_cb callback, user_data_t const *ud) { - return create_register_callback(&list_log, name, (void *)callback, ud); + return create_register_callback(&list_log, name, (placeholder_cb_t)callback, + ud); } /* int plugin_register_log */ EXPORT int plugin_register_notification(const char *name, plugin_notification_cb callback, user_data_t const *ud) { - return create_register_callback(&list_notification, name, (void *)callback, - ud); + return create_register_callback(&list_notification, name, + (placeholder_cb_t)callback, ud); } /* int plugin_register_log */ EXPORT int plugin_unregister_config(const char *name) { @@ -1742,7 +1748,7 @@ EXPORT int plugin_init_all(void) { while (le != NULL) { callback_func_t *cf = le->value; plugin_ctx_t old_ctx = plugin_set_ctx(cf->cf_ctx); - plugin_init_cb callback = (void *)cf->cf_callback; + plugin_init_cb callback = cf->cf_callback; status = (*callback)(); plugin_set_ctx(old_ctx); @@ -1806,10 +1812,10 @@ EXPORT int plugin_read_all_once(void) { old_ctx = plugin_set_ctx(rf->rf_ctx); if (rf->rf_type == RF_SIMPLE) { - int (*callback)(void) = (void *)rf->rf_callback; + int (*callback)(void) = rf->rf_callback; status = (*callback)(); } else { - plugin_read_cb callback = (void *)rf->rf_callback; + plugin_read_cb callback = (plugin_read_cb)rf->rf_callback; status = (*callback)(&rf->rf_udata); } @@ -1821,7 +1827,7 @@ EXPORT int plugin_read_all_once(void) { } sfree(rf->rf_name); - destroy_callback((void *)rf); + destroy_callback((callback_func_t *)rf); } return return_status; @@ -1872,7 +1878,7 @@ EXPORT int plugin_flush(const char *plugin, cdtime_t timeout, callback_func_t *cf = le->value; plugin_ctx_t old_ctx = plugin_set_ctx(cf->cf_ctx); - plugin_flush_cb callback = (void *)cf->cf_callback; + plugin_flush_cb callback = (plugin_flush_cb)cf->cf_callback; (*callback)(timeout, identifier, &cf->cf_udata); @@ -1913,7 +1919,7 @@ EXPORT int plugin_shutdown_all(void) { while (le != NULL) { callback_func_t *cf = le->value; plugin_ctx_t old_ctx = plugin_set_ctx(cf->cf_ctx); - plugin_shutdown_cb callback = (void *)cf->cf_callback; + plugin_shutdown_cb callback = cf->cf_callback; /* Advance the pointer before calling the callback allows * shutdown functions to unregister themselves. If done the @@ -1954,7 +1960,7 @@ EXPORT int plugin_dispatch_missing(metric_family_t const *fam) /* {{{ */ while (le != NULL) { callback_func_t *cf = le->value; plugin_ctx_t old_ctx = plugin_set_ctx(cf->cf_ctx); - plugin_missing_cb callback = cf->cf_callback; + plugin_missing_cb callback = (plugin_missing_cb)cf->cf_callback; int status = (*callback)(fam, &cf->cf_udata); plugin_set_ctx(old_ctx); @@ -2153,16 +2159,13 @@ EXPORT int plugin_dispatch_notification(const notification_t *notif) { le = llist_head(list_notification); while (le != NULL) { - callback_func_t *cf; - plugin_notification_cb callback; - int status; - /* do not switch plugin context; rather keep the context * (interval) information of the calling plugin */ - cf = le->value; - callback = cf->cf_callback; - status = (*callback)(notif, &cf->cf_udata); + callback_func_t *cf = le->value; + plugin_notification_cb callback = (plugin_notification_cb)cf->cf_callback; + + int status = (*callback)(notif, &cf->cf_udata); if (status != 0) { WARNING("plugin_dispatch_notification: Notification " "callback %s returned %i.", @@ -2198,7 +2201,7 @@ EXPORT void plugin_log(int level, const char *format, ...) { le = llist_head(list_log); while (le != NULL) { callback_func_t *cf = le->value; - plugin_log_cb callback = (void *)cf->cf_callback; + plugin_log_cb callback = (plugin_log_cb)cf->cf_callback; /* do not switch plugin context; rather keep the context * (interval) information of the calling plugin */