From: Florian Forster Date: Wed, 17 May 2017 06:40:59 +0000 (+0200) Subject: Merge remote-tracking branch 'github/pr/2277' X-Git-Tag: collectd-5.8.0~175 X-Git-Url: https://git.octo.it/?p=collectd.git;a=commitdiff_plain;h=6026e3162e522b133d10596710527d24c2921b55;hp=307c875e5a78a2729fbbe1a588d232e9a129d75a Merge remote-tracking branch 'github/pr/2277' --- diff --git a/Makefile.am b/Makefile.am index 1f18d231..d1d5071a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -691,6 +691,15 @@ curl_json_la_CFLAGS = $(AM_CFLAGS) $(BUILD_WITH_LIBCURL_CFLAGS) curl_json_la_CPPFLAGS = $(AM_CPPFLAGS) $(BUILD_WITH_LIBYAJL_CPPFLAGS) curl_json_la_LDFLAGS = $(PLUGIN_LDFLAGS) $(BUILD_WITH_LIBYAJL_LDFLAGS) curl_json_la_LIBADD = $(BUILD_WITH_LIBCURL_LIBS) $(BUILD_WITH_LIBYAJL_LIBS) + +test_plugin_curl_json_SOURCES = src/curl_json_test.c \ + src/utils_curl_stats.c \ + src/daemon/configfile.c \ + src/daemon/types_list.c +test_plugin_curl_json_CPPFLAGS = $(AM_CPPFLAGS) $(BUILD_WITH_LIBYAJL_CPPFLAGS) +test_plugin_curl_json_LDFLAGS = $(PLUGIN_LDFLAGS) $(BUILD_WITH_LIBYAJL_LDFLAGS) +test_plugin_curl_json_LDADD = libavltree.la liboconfig.la libplugin_mock.la $(BUILD_WITH_LIBCURL_LIBS) $(BUILD_WITH_LIBYAJL_LIBS) +check_PROGRAMS += test_plugin_curl_json endif if BUILD_PLUGIN_CURL_XML diff --git a/src/collectd.conf.pod b/src/collectd.conf.pod index 31b5274c..93103437 100644 --- a/src/collectd.conf.pod +++ b/src/collectd.conf.pod @@ -5865,10 +5865,10 @@ multiple hosts. =item B I Sets the interval in which to send ICMP echo packets to the configured hosts. -This is B the interval in which statistics are queries from the plugin but -the interval in which the hosts are "pinged". Therefore, the setting here -should be smaller than or equal to the global B setting. Fractional -times, such as "1.24" are allowed. +This is B the interval in which metrics are read from the plugin but the +interval in which the hosts are "pinged". Therefore, the setting here should be +smaller than or equal to the global B setting. Fractional times, such +as "1.24" are allowed. Default: B<1.0> diff --git a/src/curl_json.c b/src/curl_json.c index 9ec9f1e2..b9394c27 100644 --- a/src/curl_json.c +++ b/src/curl_json.c @@ -86,7 +86,6 @@ struct cj_s /* {{{ */ yajl_handle yajl; c_avl_tree_t *tree; - cj_key_t *key; int depth; struct { union { @@ -107,7 +106,11 @@ typedef unsigned int yajl_len_t; #endif static int cj_read(user_data_t *ud); -static void cj_submit(cj_t *db, cj_key_t *key, value_t *value); +static void cj_submit_impl(cj_t *db, cj_key_t *key, value_t *value); + +/* cj_submit is a function pointer to cj_submit_impl, allowing the unit-test to + * overwrite which function is called. */ +static void (*cj_submit)(cj_t *, cj_key_t *, value_t *) = cj_submit_impl; static size_t cj_curl_callback(void *buf, /* {{{ */ size_t size, size_t nmemb, void *user_data) { @@ -171,23 +174,53 @@ static int cj_get_type(cj_key_t *key) { return ds->ds[0].type; } -static int cj_cb_map_key(void *ctx, const unsigned char *val, yajl_len_t len); +/* cj_load_key loads the configuration for "key" from the parent context and + * sets either .key or .tree in the current context. */ +static int cj_load_key(cj_t *db, char const *key) { + if (db == NULL || key == NULL || db->depth <= 0) + return EINVAL; -static void cj_cb_inc_array_index(void *ctx, _Bool update_key) { - cj_t *db = (cj_t *)ctx; + sstrncpy(db->state[db->depth].name, key, sizeof(db->state[db->depth].name)); + + c_avl_tree_t *tree = db->state[db->depth - 1].tree; + if (tree == NULL) { + return 0; + } + + /* the parent has a key, so the tree pointer is invalid. */ + if (CJ_IS_KEY(db->state[db->depth - 1].key)) { + return 0; + } + void *value = NULL; + if (c_avl_get(tree, key, (void *)&value) == 0) { + if (CJ_IS_KEY((cj_key_t *)value)) { + db->state[db->depth].key = value; + } else { + db->state[db->depth].tree = value; + } + } else if (c_avl_get(tree, CJ_ANY, (void *)&value) == 0) { + if (CJ_IS_KEY((cj_key_t *)value)) { + db->state[db->depth].key = value; + } else { + db->state[db->depth].tree = value; + } + } else { + db->state[db->depth].key = NULL; + } + + return 0; +} + +static void cj_advance_array(cj_t *db) { if (!db->state[db->depth].in_array) return; db->state[db->depth].index++; - if (update_key) { - char name[DATA_MAX_NAME_LEN]; - - ssnprintf(name, sizeof(name), "%d", db->state[db->depth].index - 1); - - cj_cb_map_key(ctx, (unsigned char *)name, (yajl_len_t)strlen(name)); - } + char name[DATA_MAX_NAME_LEN]; + ssnprintf(name, sizeof(name), "%d", db->state[db->depth].index); + cj_load_key(db, name); } /* yajl callbacks */ @@ -195,12 +228,12 @@ static void cj_cb_inc_array_index(void *ctx, _Bool update_key) { #define CJ_CB_CONTINUE 1 static int cj_cb_boolean(void *ctx, int boolVal) { - cj_cb_inc_array_index(ctx, /* update_key = */ 0); + cj_advance_array(ctx); return CJ_CB_CONTINUE; } static int cj_cb_null(void *ctx) { - cj_cb_inc_array_index(ctx, /* update_key = */ 0); + cj_advance_array(ctx); return CJ_CB_CONTINUE; } @@ -209,40 +242,35 @@ static int cj_cb_number(void *ctx, const char *number, yajl_len_t number_len) { cj_t *db = (cj_t *)ctx; cj_key_t *key = db->state[db->depth].key; - value_t vt; - int type; - int status; /* Create a null-terminated version of the string. */ memcpy(buffer, number, number_len); buffer[sizeof(buffer) - 1] = 0; - if ((key == NULL) || !CJ_IS_KEY(key)) { - if (key != NULL && - !db->state[db->depth].in_array /*can be inhomogeneous*/) { - NOTICE("curl_json plugin: Found \"%s\", but the configuration expects" - " a map.", - buffer); - return CJ_CB_CONTINUE; - } - - cj_cb_inc_array_index(ctx, /* update_key = */ 1); - key = db->state[db->depth].key; - if ((key == NULL) || !CJ_IS_KEY(key)) { - return CJ_CB_CONTINUE; - } - } else { - cj_cb_inc_array_index(ctx, /* update_key = */ 1); + if (key == NULL) { + /* no config for this element. */ + cj_advance_array(ctx); + return CJ_CB_CONTINUE; + } else if (!CJ_IS_KEY(key)) { + /* the config expects a map or an array. */ + NOTICE( + "curl_json plugin: Found \"%s\", but the configuration expects a map.", + buffer); + cj_advance_array(ctx); + return CJ_CB_CONTINUE; } - type = cj_get_type(key); - status = parse_value(buffer, &vt, type); + int type = cj_get_type(key); + value_t vt; + int status = parse_value(buffer, &vt, type); if (status != 0) { NOTICE("curl_json plugin: Unable to parse number: \"%s\"", buffer); + cj_advance_array(ctx); return CJ_CB_CONTINUE; } cj_submit(db, key, &vt); + cj_advance_array(ctx); return CJ_CB_CONTINUE; } /* int cj_cb_number */ @@ -251,38 +279,13 @@ static int cj_cb_number(void *ctx, const char *number, yajl_len_t number_len) { * NULL. */ static int cj_cb_map_key(void *ctx, unsigned char const *in_name, yajl_len_t in_name_len) { - cj_t *db = (cj_t *)ctx; - c_avl_tree_t *tree; + char name[in_name_len + 1]; - tree = db->state[db->depth - 1].tree; - - if (tree != NULL) { - cj_key_t *value = NULL; - char *name; - size_t name_len; - - /* Create a null-terminated version of the name. */ - name = db->state[db->depth].name; - name_len = - COUCH_MIN((size_t)in_name_len, sizeof(db->state[db->depth].name) - 1); - memcpy(name, in_name, name_len); - name[name_len] = 0; - - if (c_avl_get(tree, name, (void *)&value) == 0) { - if (CJ_IS_KEY((cj_key_t *)value)) { - db->state[db->depth].key = value; - } else { - db->state[db->depth].tree = (c_avl_tree_t *)value; - } - } else if (c_avl_get(tree, CJ_ANY, (void *)&value) == 0) - if (CJ_IS_KEY((cj_key_t *)value)) { - db->state[db->depth].key = value; - } else { - db->state[db->depth].tree = (c_avl_tree_t *)value; - } - else - db->state[db->depth].key = NULL; - } + memmove(name, in_name, in_name_len); + name[sizeof(name) - 1] = 0; + + if (cj_load_key(ctx, name) != 0) + return CJ_CB_ABORT; return CJ_CB_CONTINUE; } @@ -292,38 +295,43 @@ static int cj_cb_string(void *ctx, const unsigned char *val, yajl_len_t len) { return cj_cb_number(ctx, (const char *)val, len); } /* int cj_cb_string */ -static int cj_cb_start(void *ctx) { - cj_t *db = (cj_t *)ctx; - if (++db->depth >= YAJL_MAX_DEPTH) { - ERROR("curl_json plugin: %s depth exceeds max, aborting.", - db->url ? db->url : db->sock); - return CJ_CB_ABORT; - } - return CJ_CB_CONTINUE; -} - static int cj_cb_end(void *ctx) { cj_t *db = (cj_t *)ctx; db->state[db->depth].tree = NULL; - --db->depth; + db->depth--; + cj_advance_array(ctx); return CJ_CB_CONTINUE; } static int cj_cb_start_map(void *ctx) { - cj_cb_inc_array_index(ctx, /* update_key = */ 1); - return cj_cb_start(ctx); + cj_t *db = (cj_t *)ctx; + + if ((db->depth + 1) >= YAJL_MAX_DEPTH) { + ERROR("curl_json plugin: %s depth exceeds max, aborting.", + db->url ? db->url : db->sock); + return CJ_CB_ABORT; + } + db->depth++; + return CJ_CB_CONTINUE; } static int cj_cb_end_map(void *ctx) { return cj_cb_end(ctx); } static int cj_cb_start_array(void *ctx) { cj_t *db = (cj_t *)ctx; - cj_cb_inc_array_index(ctx, /* update_key = */ 1); - if (db->depth + 1 < YAJL_MAX_DEPTH) { - db->state[db->depth + 1].in_array = 1; - db->state[db->depth + 1].index = 0; + + if ((db->depth + 1) >= YAJL_MAX_DEPTH) { + ERROR("curl_json plugin: %s depth exceeds max, aborting.", + db->url ? db->url : db->sock); + return CJ_CB_ABORT; } - return cj_cb_start(ctx); + db->depth++; + db->state[db->depth].in_array = 1; + db->state[db->depth].index = 0; + + cj_load_key(db, "0"); + + return CJ_CB_CONTINUE; } static int cj_cb_end_array(void *ctx) { @@ -412,7 +420,7 @@ static void cj_free(void *arg) /* {{{ */ /* Configuration handling functions {{{ */ static c_avl_tree_t *cj_avl_create(void) { - return c_avl_create((int(*)(const void *, const void *))strcmp); + return c_avl_create((int (*)(const void *, const void *))strcmp); } static int cj_config_append_string(const char *name, @@ -433,6 +441,53 @@ static int cj_config_append_string(const char *name, return 0; } /* }}} int cj_config_append_string */ +/* cj_append_key adds key to the configuration stored in db. + * + * For example: + * "httpd/requests/count", + * "httpd/requests/current" -> + * { "httpd": { "requests": { "count": $key, "current": $key } } } + */ +static int cj_append_key(cj_t *db, cj_key_t *key) { /* {{{ */ + if (db->tree == NULL) + db->tree = cj_avl_create(); + + c_avl_tree_t *tree = db->tree; + + char const *start = key->path; + if (*start == '/') + ++start; + + char const *end; + while ((end = strchr(start, '/')) != NULL) { + char name[PATH_MAX]; + + size_t len = end - start; + if (len == 0) + break; + + len = COUCH_MIN(len, sizeof(name) - 1); + sstrncpy(name, start, len + 1); + + c_avl_tree_t *value; + if (c_avl_get(tree, name, (void *)&value) != 0) { + value = cj_avl_create(); + c_avl_insert(tree, strdup(name), value); + } + + tree = value; + start = end + 1; + } + + if (strlen(start) == 0) { + ERROR("curl_json plugin: invalid key: %s", key->path); + return -1; + } + + c_avl_insert(tree, strdup(start), key); + return 0; +} /* }}} int cj_append_key */ + static int cj_config_add_key(cj_t *db, /* {{{ */ oconfig_item_t *ci) { cj_key_t *key; @@ -493,53 +548,13 @@ static int cj_config_add_key(cj_t *db, /* {{{ */ return -1; } - /* store path in a tree that will match the json map structure, example: - * "httpd/requests/count", - * "httpd/requests/current" -> - * { "httpd": { "requests": { "count": $key, "current": $key } } } - */ - char *ptr; - char *name; - c_avl_tree_t *tree; - - if (db->tree == NULL) - db->tree = cj_avl_create(); - - tree = db->tree; - ptr = key->path; - if (*ptr == '/') - ++ptr; - - name = ptr; - while ((ptr = strchr(name, '/')) != NULL) { - char ent[PATH_MAX]; - c_avl_tree_t *value; - size_t len; - - len = ptr - name; - if (len == 0) - break; - - len = COUCH_MIN(len, sizeof(ent) - 1); - sstrncpy(ent, name, len + 1); - - if (c_avl_get(tree, ent, (void *)&value) != 0) { - value = cj_avl_create(); - c_avl_insert(tree, strdup(ent), value); - } - - tree = value; - name = ptr + 1; - } - - if (strlen(name) == 0) { - ERROR("curl_json plugin: invalid key: %s", key->path); + status = cj_append_key(db, key); + if (status != 0) { cj_key_free(key); return -1; } - c_avl_insert(tree, strdup(name), key); - return status; + return 0; } /* }}} int cj_config_add_key */ static int cj_init_curl(cj_t *db) /* {{{ */ @@ -767,7 +782,7 @@ static const char *cj_host(cj_t *db) /* {{{ */ return db->host; } /* }}} cj_host */ -static void cj_submit(cj_t *db, cj_key_t *key, value_t *value) /* {{{ */ +static void cj_submit_impl(cj_t *db, cj_key_t *key, value_t *value) /* {{{ */ { value_list_t vl = VALUE_LIST_INIT; @@ -791,13 +806,14 @@ static void cj_submit(cj_t *db, cj_key_t *key, value_t *value) /* {{{ */ vl.interval = db->interval; plugin_dispatch_values(&vl); -} /* }}} int cj_submit */ +} /* }}} int cj_submit_impl */ static int cj_sock_perform(cj_t *db) /* {{{ */ { char errbuf[1024]; - struct sockaddr_un sa_unix = {0}; - sa_unix.sun_family = AF_UNIX; + struct sockaddr_un sa_unix = { + .sun_family = AF_UNIX, + }; sstrncpy(sa_unix.sun_path, db->sock, sizeof(sa_unix.sun_path)); int fd = socket(AF_UNIX, SOCK_STREAM, 0); @@ -923,7 +939,6 @@ static int cj_read(user_data_t *ud) /* {{{ */ db->depth = 0; memset(&db->state, 0, sizeof(db->state)); db->state[db->depth].tree = db->tree; - db->key = NULL; return cj_perform(db); } /* }}} int cj_read */ diff --git a/src/curl_json_test.c b/src/curl_json_test.c new file mode 100644 index 00000000..1e4d3b3b --- /dev/null +++ b/src/curl_json_test.c @@ -0,0 +1,144 @@ +/** + * collectd - src/curl_json.c + * Copyright (C) 2017 Florian octo Forster + * + * Licensed under the same terms and conditions as src/curl_json.c. + * + * Authors: + * Florian octo Forster + **/ + +#include "curl_json.c" + +#include "testing.h" + +static void test_submit(cj_t *db, cj_key_t *key, value_t *value) { + /* hack: we repurpose db->curl to store received values. */ + c_avl_tree_t *values = (void *)db->curl; + + value_t *value_copy = calloc(1, sizeof(*value_copy)); + memmove(value_copy, value, sizeof(*value_copy)); + + assert(c_avl_insert(values, key->path, value_copy) == 0); +} + +static derive_t test_metric(cj_t *db, char const *path) { + c_avl_tree_t *values = (void *)db->curl; + + value_t *ret = NULL; + if (c_avl_get(values, path, (void *)&ret) == 0) { + return ret->derive; + } + + return -1; +} + +static cj_t *test_setup(char *json, char *key_path) { + cj_t *db = calloc(1, sizeof(*db)); + db->yajl = yajl_alloc(&ycallbacks, +#if HAVE_YAJL_V2 + /* alloc funcs = */ NULL, +#else + /* alloc funcs = */ NULL, NULL, +#endif + /* context = */ (void *)db); + + /* hack; see above. */ + db->curl = (void *)cj_avl_create(); + + cj_key_t *key = calloc(1, sizeof(*key)); + key->magic = CJ_KEY_MAGIC; + key->path = strdup(key_path); + key->type = strdup("MAGIC"); + + assert(cj_append_key(db, key) == 0); + + db->state[0].tree = db->tree; + + cj_curl_callback(json, strlen(json), 1, db); +#if HAVE_YAJL_V2 + yajl_complete_parse(db->yajl); +#else + yajl_parse_complete(db->yajl); +#endif + + return db; +} + +static void test_teardown(cj_t *db) { + c_avl_tree_t *values = (void *)db->curl; + db->curl = NULL; + + void *key; + void *value; + while (c_avl_pick(values, &key, &value) == 0) { + /* key will be freed by cj_free. */ + free(value); + } + c_avl_destroy(values); + + yajl_free(db->yajl); + db->yajl = NULL; + + cj_free(db); +} + +DEF_TEST(parse) { + struct { + char *json; + char *key_path; + derive_t want; + } cases[] = { + /* simple map */ + {"{\"foo\":42,\"bar\":23}", "foo", 42}, + {"{\"foo\":42,\"bar\":23}", "bar", 23}, + /* nested map */ + {"{\"a\":{\"b\":{\"c\":123}}", "a/b/c", 123}, + {"{\"x\":{\"y\":{\"z\":789}}", "x/*/z", 789}, + /* simple array */ + {"[10,11,12,13]", "0", 10}, + {"[10,11,12,13]", "1", 11}, + {"[10,11,12,13]", "2", 12}, + {"[10,11,12,13]", "3", 13}, + /* array index after non-numeric entry */ + {"[true,11]", "1", 11}, + {"[null,11]", "1", 11}, + {"[\"s\",11]", "1", 11}, + {"[{\"k\":\"v\"},11]", "1", 11}, + {"[[0,1,2],11]", "1", 11}, + /* nested array */ + {"[[0,1,2],[3,4,5],[6,7,8]]", "0/0", 0}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "0/1", 1}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "0/2", 2}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "1/0", 3}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "1/1", 4}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "1/2", 5}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "2/0", 6}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "2/1", 7}, + {"[[0,1,2],[3,4,5],[6,7,8]]", "2/2", 8}, + /* testcase from #2266 */ + {"{\"a\":[[10,11,12,13,14]]}", "a/0/0", 10}, + {"{\"a\":[[10,11,12,13,14]]}", "a/0/1", 11}, + {"{\"a\":[[10,11,12,13,14]]}", "a/0/2", 12}, + {"{\"a\":[[10,11,12,13,14]]}", "a/0/3", 13}, + {"{\"a\":[[10,11,12,13,14]]}", "a/0/4", 14}, + }; + + for (size_t i = 0; i < STATIC_ARRAY_SIZE(cases); i++) { + cj_t *db = test_setup(cases[i].json, cases[i].key_path); + + EXPECT_EQ_INT(cases[i].want, test_metric(db, cases[i].key_path)); + + test_teardown(db); + } + + return 0; +} + +int main(int argc, char **argv) { + cj_submit = test_submit; + + RUN_TEST(parse); + + END_TEST; +} diff --git a/src/daemon/plugin_mock.c b/src/daemon/plugin_mock.c index 9e7ba39e..ca985398 100644 --- a/src/daemon/plugin_mock.c +++ b/src/daemon/plugin_mock.c @@ -32,6 +32,11 @@ kstat_ctl_t *kc = NULL; char hostname_g[] = "example.com"; +void plugin_set_dir(const char *dir) { /* nop */ +} + +int plugin_load(const char *name, _Bool global) { return ENOTSUP; } + int plugin_register_config(const char *name, int (*callback)(const char *key, const char *val), const char **keys, int keys_num) { @@ -62,6 +67,8 @@ int plugin_register_shutdown(const char *name, int (*callback)(void)) { return ENOTSUP; } +int plugin_register_data_set(const data_set_t *ds) { return ENOTSUP; } + int plugin_dispatch_values(value_list_t const *vl) { return ENOTSUP; } int plugin_flush(const char *plugin, cdtime_t timeout, const char *identifier) { @@ -88,4 +95,25 @@ void plugin_log(int level, char const *format, ...) { printf("plugin_log (%i, \"%s\");\n", level, buffer); } -cdtime_t plugin_get_interval(void) { return TIME_T_TO_CDTIME_T(10); } +void plugin_init_ctx(void) { /* nop */ +} + +plugin_ctx_t mock_context = { + .interval = TIME_T_TO_CDTIME_T_STATIC(10), +}; + +plugin_ctx_t plugin_get_ctx(void) { return mock_context; } + +plugin_ctx_t plugin_set_ctx(plugin_ctx_t ctx) { + plugin_ctx_t prev = mock_context; + mock_context = ctx; + return prev; +} + +cdtime_t plugin_get_interval(void) { return mock_context.interval; } + +/* TODO(octo): this function is actually from filter_chain.h, but in order not + * to tumble down that rabbit hole, we're declaring it here. A better solution + * would be to hard-code the top-level config keys in daemon/collectd.c to avoid + * having these references in daemon/configfile.c. */ +int fc_configure(const oconfig_item_t *ci) { return ENOTSUP; } diff --git a/src/perl.c b/src/perl.c index 9f61928f..e922badf 100644 --- a/src/perl.c +++ b/src/perl.c @@ -494,16 +494,15 @@ static int av2data_set(pTHX_ AV *array, char *name, data_set_t *ds) { * meta => [ { name => , value => }, ... ] * } */ -static int av2notification_meta(pTHX_ AV *array, notification_meta_t **meta) { - notification_meta_t **m = meta; +static int av2notification_meta(pTHX_ AV *array, notification_meta_t **ret_meta) { + notification_meta_t *tail = NULL; int len = av_len(array); for (int i = 0; i <= len; ++i) { SV **tmp = av_fetch(array, i, 0); - HV *hash; - if (NULL == tmp) + if (tmp == NULL) return -1; if (!(SvROK(*tmp) && (SVt_PVHV == SvTYPE(SvRV(*tmp))))) { @@ -512,42 +511,51 @@ static int av2notification_meta(pTHX_ AV *array, notification_meta_t **meta) { continue; } - hash = (HV *)SvRV(*tmp); + HV *hash = (HV *)SvRV(*tmp); - *m = smalloc(sizeof(**m)); + notification_meta_t *m = calloc(1, sizeof(*m)); + if (m == NULL) + return ENOMEM; - if (NULL == (tmp = hv_fetch(hash, "name", 4, 0))) { + SV **name = hv_fetch(hash, "name", strlen("name"), 0); + if (name == NULL) { log_warn("av2notification_meta: Skipping invalid " "meta information."); - free(*m); + sfree(m); continue; } - sstrncpy((*m)->name, SvPV_nolen(*tmp), sizeof((*m)->name)); + sstrncpy(m->name, SvPV_nolen(*name), sizeof(m->name)); - if (NULL == (tmp = hv_fetch(hash, "value", 5, 0))) { + SV **value = hv_fetch(hash, "value", strlen("value"), 0); + if (value == NULL) { log_warn("av2notification_meta: Skipping invalid " "meta information."); - free(*m); + sfree(m); continue; } - if (SvNOK(*tmp)) { - (*m)->nm_value.nm_double = SvNVX(*tmp); - (*m)->type = NM_TYPE_DOUBLE; - } else if (SvUOK(*tmp)) { - (*m)->nm_value.nm_unsigned_int = SvUVX(*tmp); - (*m)->type = NM_TYPE_UNSIGNED_INT; - } else if (SvIOK(*tmp)) { - (*m)->nm_value.nm_signed_int = SvIVX(*tmp); - (*m)->type = NM_TYPE_SIGNED_INT; + if (SvNOK(*value)) { + m->nm_value.nm_double = SvNVX(*value); + m->type = NM_TYPE_DOUBLE; + } else if (SvUOK(*value)) { + m->nm_value.nm_unsigned_int = SvUVX(*value); + m->type = NM_TYPE_UNSIGNED_INT; + } else if (SvIOK(*value)) { + m->nm_value.nm_signed_int = SvIVX(*value); + m->type = NM_TYPE_SIGNED_INT; } else { - (*m)->nm_value.nm_string = sstrdup(SvPV_nolen(*tmp)); - (*m)->type = NM_TYPE_STRING; + m->nm_value.nm_string = sstrdup(SvPV_nolen(*value)); + m->type = NM_TYPE_STRING; } - (*m)->next = NULL; - m = &((*m)->next); + m->next = NULL; + if (tail == NULL) + *ret_meta = m; + else + tail->next = m; + tail = m; } + return 0; } /* static int av2notification_meta (AV *, notification_meta_t *) */ diff --git a/src/testing.h b/src/testing.h index a89f4629..d3da9db4 100644 --- a/src/testing.h +++ b/src/testing.h @@ -104,7 +104,8 @@ static int check_count__ = 0; do { \ double want__ = (double)expect; \ double got__ = (double)actual; \ - if (isnan(want__) && !isnan(got__)) { \ + if ((isnan(want__) && !isnan(got__)) || \ + (!isnan(want__) && isnan(got__))) { \ printf("not ok %i - %s = %.15g, want %.15g\n", ++check_count__, #actual, \ got__, want__); \ return -1; \ diff --git a/src/valgrind.FreeBSD.suppress b/src/valgrind.FreeBSD.suppress index 7a238173..862085d7 100644 --- a/src/valgrind.FreeBSD.suppress +++ b/src/valgrind.FreeBSD.suppress @@ -2,7 +2,6 @@ strlen_bogus_invalid_read_after_strdup Memcheck:Addr4 fun:parse_value - fun:parse_values ... fun:main } diff --git a/src/virt.c b/src/virt.c index a8f378a2..14095df4 100644 --- a/src/virt.c +++ b/src/virt.c @@ -514,7 +514,8 @@ static int lv_domain_info(virDomainPtr dom, struct lv_info *info) { ret = virDomainGetCPUStats(dom, param, nparams, -1, 1, 0); // total stats. if (ret < 0) { - virTypedParamsFree(param, nparams); + virTypedParamsClear(param, nparams); + sfree(param); VIRT_ERROR(conn, "getting the disk params values"); return -1; } @@ -526,7 +527,8 @@ static int lv_domain_info(virDomainPtr dom, struct lv_info *info) { info->total_syst_cpu_time = param[i].value.ul; } - virTypedParamsFree(param, nparams); + virTypedParamsClear(param, nparams); + sfree(param); #endif /* HAVE_CPU_STATS */ return 0; @@ -1110,31 +1112,29 @@ static void lv_disconnect(void) { static int lv_domain_block_info(virDomainPtr dom, const char *path, struct lv_block_info *binfo) { #ifdef HAVE_BLOCK_STATS_FLAGS - virTypedParameterPtr params = NULL; int nparams = 0; - int rc = -1; - int ret = virDomainBlockStatsFlags(dom, path, NULL, &nparams, 0); - if (ret < 0 || nparams == 0) { + if (virDomainBlockStatsFlags(dom, path, NULL, &nparams, 0) < 0 || + nparams <= 0) { VIRT_ERROR(conn, "getting the disk params count"); return -1; } - params = calloc(nparams, sizeof(virTypedParameter)); + virTypedParameterPtr params = calloc((size_t)nparams, sizeof(*params)); if (params == NULL) { ERROR("virt plugin: alloc(%i) for block=%s parameters failed.", nparams, path); return -1; } - ret = virDomainBlockStatsFlags(dom, path, params, &nparams, 0); - if (ret < 0) { + + int rc = -1; + if (virDomainBlockStatsFlags(dom, path, params, &nparams, 0) < 0) { VIRT_ERROR(conn, "getting the disk params values"); - goto done; + } else { + rc = get_block_info(binfo, params, nparams); } - rc = get_block_info(binfo, params, nparams); - -done: - virTypedParamsFree(params, nparams); + virTypedParamsClear(params, nparams); + sfree(params); return rc; #else return virDomainBlockStats(dom, path, &(binfo->bi), sizeof(binfo->bi)); @@ -1802,9 +1802,9 @@ static int lv_instance_include_domain(struct lv_read_instance *inst, we can't detect this. */ #ifdef LIBVIR_CHECK_VERSION -# if LIBVIR_CHECK_VERSION(0,10,2) -# define HAVE_LIST_ALL_DOMAINS 1 -# endif +#if LIBVIR_CHECK_VERSION(0, 10, 2) +#define HAVE_LIST_ALL_DOMAINS 1 +#endif #endif static int refresh_lists(struct lv_read_instance *inst) { @@ -1822,7 +1822,8 @@ static int refresh_lists(struct lv_read_instance *inst) { if (n > 0) { #ifdef HAVE_LIST_ALL_DOMAINS virDomainPtr *domains; - n = virConnectListAllDomains (conn, &domains, VIR_CONNECT_LIST_DOMAINS_ACTIVE); + n = virConnectListAllDomains(conn, &domains, + VIR_CONNECT_LIST_DOMAINS_ACTIVE); #else int *domids; @@ -2007,7 +2008,7 @@ static int refresh_lists(struct lv_read_instance *inst) { } #ifdef HAVE_LIST_ALL_DOMAINS - sfree (domains); + sfree(domains); #else sfree(domids); #endif