From 4a8d2bfc2b5370e57a9587b9e16e2f4d17e0799c Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 11 Jun 2019 18:32:58 +0200 Subject: [PATCH 1/2] hooks: use exceptions for error handling --- include/villas/hook_list.hpp | 10 +++---- lib/hook_list.cpp | 58 ++++++++---------------------------- lib/node_direction.cpp | 21 +++---------- lib/path.cpp | 12 ++------ 4 files changed, 25 insertions(+), 76 deletions(-) diff --git a/include/villas/hook_list.hpp b/include/villas/hook_list.hpp index 522bc2dc8..7706c46dc 100644 --- a/include/villas/hook_list.hpp +++ b/include/villas/hook_list.hpp @@ -57,9 +57,9 @@ int hook_list_destroy(struct vlist *hs); * hooks = [ "print" ] * } */ -int hook_list_parse(struct vlist *hs, json_t *cfg, int mask, struct path *p, struct node *n); +void hook_list_parse(struct vlist *hs, json_t *cfg, int mask, struct path *p, struct node *n); -int hook_list_prepare(struct vlist *hs, struct vlist *sigs, int mask, struct path *p, struct node *n); +void hook_list_prepare(struct vlist *hs, struct vlist *sigs, int mask, struct path *p, struct node *n); int hook_list_prepare_signals(struct vlist *hs, struct vlist *signals); @@ -67,10 +67,10 @@ int hook_list_add(struct vlist *hs, int mask, struct path *p, struct node *n); int hook_list_process(struct vlist *hs, struct sample *smps[], unsigned cnt); -int hook_list_periodic(struct vlist *hs); +void hook_list_periodic(struct vlist *hs); -int hook_list_start(struct vlist *hs); +void hook_list_start(struct vlist *hs); -int hook_list_stop(struct vlist *hs); +void hook_list_stop(struct vlist *hs); struct vlist * hook_list_get_signals(struct vlist *hs); diff --git a/lib/hook_list.cpp b/lib/hook_list.cpp index 1124997d7..18116b567 100644 --- a/lib/hook_list.cpp +++ b/lib/hook_list.cpp @@ -58,11 +58,10 @@ int hook_list_destroy(vlist *hs) return 0; } -int hook_list_parse(vlist *hs, json_t *cfg, int mask, struct path *o, struct node *n) +void hook_list_parse(vlist *hs, json_t *cfg, int mask, struct path *o, struct node *n) { if (!json_is_array(cfg)) - //throw ConfigError(cfg, "node-config-hook", "Hooks must be configured as a list of hook objects"); - return -1; + throw ConfigError(cfg, "node-config-hook", "Hooks must be configured as a list of hook objects"); size_t i; json_t *json_hook; @@ -83,19 +82,12 @@ int hook_list_parse(vlist *hs, json_t *cfg, int mask, struct path *o, struct nod if (!(hf->getFlags() & mask)) throw ConfigError(json_hook, "node-config-hook", "Hook '{}' not allowed here", type); - try { - h = hf->make(o, n); - h->parse(json_hook); - h->check(); - } - catch (...) { - return -1; - } + h = hf->make(o, n); + h->parse(json_hook); + h->check(); vlist_push(hs, h); } - - return 0; } static int hook_cmp_priority(const Hook *a, const Hook *b) @@ -108,7 +100,7 @@ static int hook_is_enabled(const Hook *h) return h->isEnabled() ? 0 : -1; } -int hook_list_prepare(vlist *hs, vlist *sigs, int m, struct path *p, struct node *n) +void hook_list_prepare(vlist *hs, vlist *sigs, int m, struct path *p, struct node *n) { assert(hs->state == STATE_INITIALIZED); @@ -134,16 +126,10 @@ skip_add: for (size_t i = 0; i < vlist_length(hs); i++) { Hook *h = (Hook *) vlist_at(hs, i); - try { - h->prepare(sigs); - } catch (...) { - return -1; - } + h->prepare(sigs); sigs = h->getSignals(); } - - return 0; } int hook_list_process(vlist *hs, sample *smps[], unsigned cnt) @@ -182,49 +168,31 @@ skip: {} stop: return processed; } -int hook_list_periodic(vlist *hs) +void hook_list_periodic(vlist *hs) { for (size_t j = 0; j < vlist_length(hs); j++) { Hook *h = (Hook *) vlist_at(hs, j); - try { - h->periodic(); - } catch (...) { - return -1; - } + h->periodic(); } - - return 0; } -int hook_list_start(vlist *hs) +void hook_list_start(vlist *hs) { for (size_t i = 0; i < vlist_length(hs); i++) { Hook *h = (Hook *) vlist_at(hs, i); - try { - h->start(); - } catch (...) { - return -1; - } + h->start(); } - - return 0; } -int hook_list_stop(vlist *hs) +void hook_list_stop(vlist *hs) { for (size_t i = 0; i < vlist_length(hs); i++) { Hook *h = (Hook *) vlist_at(hs, i); - try { - h->stop(); - } catch (...) { - return -1; - } + h->stop(); } - - return 0; } vlist * hook_list_get_signals(vlist *hs) diff --git a/lib/node_direction.cpp b/lib/node_direction.cpp index d5db528fa..130d12f8c 100644 --- a/lib/node_direction.cpp +++ b/lib/node_direction.cpp @@ -35,13 +35,10 @@ int node_direction_prepare(struct node_direction *nd, struct node *n) assert(nd->state == STATE_CHECKED); #ifdef WITH_HOOKS - int ret; int t = nd->direction == NODE_DIR_OUT ? HOOK_NODE_WRITE : HOOK_NODE_READ; int m = nd->builtin ? t | HOOK_BUILTIN : 0; - ret = hook_list_prepare(&nd->hooks, &nd->signals, m, nullptr, n); - if (ret) - return ret; + hook_list_prepare(&nd->hooks, &nd->signals, m, nullptr, n); #endif /* WITH_HOOKS */ nd->state = STATE_PREPARED; @@ -161,9 +158,7 @@ int node_direction_parse(struct node_direction *nd, struct node *n, json_t *cfg) if (json_hooks) { int m = nd->direction == NODE_DIR_OUT ? HOOK_NODE_WRITE : HOOK_NODE_READ; - ret = hook_list_parse(&nd->hooks, json_hooks, m, nullptr, n); - if (ret < 0) - return ret; + hook_list_parse(&nd->hooks, json_hooks, m, nullptr, n); } #endif /* WITH_HOOKS */ @@ -193,11 +188,7 @@ int node_direction_start(struct node_direction *nd, struct node *n) assert(nd->state == STATE_PREPARED); #ifdef WITH_HOOKS - int ret; - - ret = hook_list_start(&nd->hooks); - if (ret) - return ret; + hook_list_start(&nd->hooks); #endif /* WITH_HOOKS */ nd->state = STATE_STARTED; @@ -210,11 +201,7 @@ int node_direction_stop(struct node_direction *nd, struct node *n) assert(nd->state == STATE_STARTED); #ifdef WITH_HOOKS - int ret; - - ret = hook_list_stop(&nd->hooks); - if (ret) - return ret; + hook_list_stop(&nd->hooks); #endif /* WITH_HOOKS */ nd->state = STATE_STOPPED; diff --git a/lib/path.cpp b/lib/path.cpp index 0963945bf..f2904773d 100644 --- a/lib/path.cpp +++ b/lib/path.cpp @@ -284,9 +284,7 @@ int path_prepare(struct path *p) int m = p->builtin ? HOOK_PATH | HOOK_BUILTIN : 0; /* Add internal hooks if they are not already in the list */ - ret = hook_list_prepare(&p->hooks, &p->signals, m, p, nullptr); - if (ret) - return ret; + hook_list_prepare(&p->hooks, &p->signals, m, p, nullptr); #endif /* WITH_HOOKS */ /* Initialize pool */ @@ -610,9 +608,7 @@ int path_start(struct path *p) ); #ifdef WITH_HOOKS - ret = hook_list_start(&p->hooks); - if (ret) - return ret; + hook_list_start(&p->hooks); #endif /* WITH_HOOKS */ p->last_sequence = 0; @@ -675,9 +671,7 @@ int path_stop(struct path *p) return ret; #ifdef WITH_HOOKS - ret = hook_list_stop(&p->hooks); - if (ret) - return ret; + hook_list_stop(&p->hooks); #endif /* WITH_HOOKS */ sample_decref(p->last_sample); From 6b2e53feda0974038b6ab88de788e5ce90ad8611 Mon Sep 17 00:00:00 2001 From: Steffen Vogel Date: Tue, 11 Jun 2019 18:34:23 +0200 Subject: [PATCH 2/2] path: keep a per path list of mappings --- include/villas/path.h | 1 + lib/path.cpp | 32 ++++++++++++++++---------------- lib/path_source.cpp | 2 +- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/include/villas/path.h b/include/villas/path.h index 5775c86d0..50eabcf50 100644 --- a/include/villas/path.h +++ b/include/villas/path.h @@ -70,6 +70,7 @@ struct path { struct vlist sources; /**< List of all incoming nodes (struct path_source). */ struct vlist destinations; /**< List of all outgoing nodes (struct path_destination). */ + struct vlist mappings; /**< List of all input mappings (struct mapping_entry). */ struct vlist hooks; /**< List of processing hooks (struct hook). */ struct vlist signals; /**< List of signals which this path creates (struct signal). */ diff --git a/lib/path.cpp b/lib/path.cpp index f2904773d..7e70833d9 100644 --- a/lib/path.cpp +++ b/lib/path.cpp @@ -135,6 +135,10 @@ int path_init(struct path *p) if (ret) return ret; + ret = vlist_init(&p->mappings); + if (ret) + return ret; + #ifdef WITH_HOOKS ret = hook_list_init(&p->hooks); if (ret) @@ -233,6 +237,10 @@ int path_prepare(struct path *p) } /* Initialize sources */ + ret = mapping_list_prepare(&p->mappings); + if (ret) + return ret; + for (size_t i = 0; i < vlist_length(&p->sources); i++) { struct path_source *ps = (struct path_source *) vlist_at(&p->sources, i); @@ -243,10 +251,6 @@ int path_prepare(struct path *p) if (ps->masked) p->mask.set(i); - ret = mapping_list_prepare(&ps->mappings); - if (ret) - return ret; - for (size_t i = 0; i < vlist_length(&ps->mappings); i++) { struct mapping_entry *me = (struct mapping_entry *) vlist_at(&ps->mappings, i); struct vlist *sigs = node_get_signals(me->node, NODE_DIR_IN); @@ -319,10 +323,8 @@ int path_parse(struct path *p, json_t *cfg, struct vlist *nodes) const char *mode = nullptr; - struct vlist sources = { .state = STATE_DESTROYED }; struct vlist destinations = { .state = STATE_DESTROYED }; - vlist_init(&sources); vlist_init(&destinations); ret = json_unpack_ex(cfg, &err, 0, "{ s: o, s?: o, s?: o, s?: b, s?: b, s?: b, s?: i, s?: s, s?: b, s?: F, s?: o, s?: b}", @@ -343,7 +345,7 @@ int path_parse(struct path *p, json_t *cfg, struct vlist *nodes) jerror(&err, "Failed to parse path configuration"); /* Input node(s) */ - ret = mapping_list_parse(&sources, json_in, nodes); + ret = mapping_list_parse(&p->mappings, json_in, nodes); if (ret) { p->logger->error("Failed to parse input mapping of path {}", path_name(p)); return -1; @@ -368,8 +370,8 @@ int path_parse(struct path *p, json_t *cfg, struct vlist *nodes) jerror(&err, "Failed to parse output nodes"); } - for (size_t i = 0; i < vlist_length(&sources); i++) { - struct mapping_entry *me = (struct mapping_entry *) vlist_at(&sources, i); + for (size_t i = 0; i < vlist_length(&p->mappings); i++) { + struct mapping_entry *me = (struct mapping_entry *) vlist_at(&p->mappings, i); struct path_source *ps = nullptr; /* Check if there is already a path_source for this source */ @@ -474,9 +476,7 @@ int path_parse(struct path *p, json_t *cfg, struct vlist *nodes) #ifdef WITH_HOOKS if (json_hooks) { - ret = hook_list_parse(&p->hooks, json_hooks, HOOK_PATH, p, nullptr); - if (ret) - return ret; + hook_list_parse(&p->hooks, json_hooks, HOOK_PATH, p, nullptr); } #endif /* WITH_HOOKS */ @@ -490,10 +490,6 @@ int path_parse(struct path *p, json_t *cfg, struct vlist *nodes) p->poll = 0; } - ret = vlist_destroy(&sources, nullptr, false); - if (ret) - return ret; - ret = vlist_destroy(&destinations, nullptr, false); if (ret) return ret; @@ -705,6 +701,10 @@ int path_destroy(struct path *p) if (ret) return ret; + ret = vlist_destroy(&p->mappings, nullptr, true); + if (ret) + return ret; + if (p->reader.pfds) free(p->reader.pfds); diff --git a/lib/path_source.cpp b/lib/path_source.cpp index b82378840..4a289ff14 100644 --- a/lib/path_source.cpp +++ b/lib/path_source.cpp @@ -52,7 +52,7 @@ int path_source_destroy(struct path_source *ps) if (ret) return ret; - ret = vlist_destroy(&ps->mappings, nullptr, true); + ret = vlist_destroy(&ps->mappings, nullptr, false); if (ret) return ret;