From d2a80178a8de377e4e270e2e84fdcb23eb057437 Mon Sep 17 00:00:00 2001 From: Igor Peshansky Date: Sun, 11 Sep 2016 19:39:58 -0400 Subject: [PATCH] Address review comments: - Code simplifications, - Leak fixes, - ERROR -> WARNING. --- src/target_replace.c | 116 ++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 56 deletions(-) diff --git a/src/target_replace.c b/src/target_replace.c index 975acbce..dba3a8cf 100644 --- a/src/target_replace.c +++ b/src/target_replace.c @@ -38,7 +38,7 @@ struct tr_action_s { regex_t re; char *replacement; - int may_be_empty; + _Bool may_be_empty; tr_action_t *next; }; @@ -113,7 +113,7 @@ static void tr_meta_data_action_destroy (tr_meta_data_action_t *act) /* {{{ */ } /* }}} void tr_meta_data_action_destroy */ static int tr_config_add_action (tr_action_t **dest, /* {{{ */ - const oconfig_item_t *ci, int may_be_empty) + const oconfig_item_t *ci, _Bool may_be_empty) { tr_action_t *act; int status; @@ -158,8 +158,7 @@ static int tr_config_add_action (tr_action_t **dest, /* {{{ */ if (act->replacement == NULL) { ERROR ("tr_config_add_action: tr_strdup failed."); - regfree (&act->re); - sfree (act); + tr_action_destroy (act); return (-ENOMEM); } @@ -181,7 +180,7 @@ static int tr_config_add_action (tr_action_t **dest, /* {{{ */ } /* }}} int tr_config_add_action */ static int tr_config_add_meta_action (tr_meta_data_action_t **dest, /* {{{ */ - const oconfig_item_t *ci, int should_delete) + const oconfig_item_t *ci, _Bool should_delete) { tr_meta_data_action_t *act; int status; @@ -230,14 +229,6 @@ static int tr_config_add_meta_action (tr_meta_data_action_t **dest, /* {{{ */ act->key = NULL; act->replacement = NULL; - act->key = tr_strdup (ci->values[0].value.string); - if (act->key == NULL) - { - ERROR ("tr_config_add_meta_action: tr_strdup failed."); - sfree (act); - return (-ENOMEM); - } - status = regcomp (&act->re, ci->values[1].value.string, REG_EXTENDED); if (status != 0) { @@ -253,14 +244,20 @@ static int tr_config_add_meta_action (tr_meta_data_action_t **dest, /* {{{ */ return (-EINVAL); } + act->key = tr_strdup (ci->values[0].value.string); + if (act->key == NULL) + { + ERROR ("tr_config_add_meta_action: tr_strdup failed."); + tr_meta_data_action_destroy (act); + return (-ENOMEM); + } + if (!should_delete) { act->replacement = tr_strdup (ci->values[2].value.string); if (act->replacement == NULL) { ERROR ("tr_config_add_meta_action: tr_strdup failed."); - sfree (act->key); - regfree (&act->re); - sfree (act); + tr_meta_data_action_destroy (act); return (-ENOMEM); } } @@ -283,7 +280,7 @@ static int tr_config_add_meta_action (tr_meta_data_action_t **dest, /* {{{ */ } /* }}} int tr_config_add_meta_action */ static int tr_action_invoke (tr_action_t *act_head, /* {{{ */ - char *buffer_in, size_t buffer_in_size, int may_be_empty) + char *buffer_in, size_t buffer_in_size, _Bool may_be_empty) { int status; char buffer[DATA_MAX_NAME_LEN]; @@ -363,13 +360,14 @@ static int tr_meta_data_action_invoke ( /* {{{ */ int value_type; int meta_data_status; char *value; + meta_data_t *result; value_type = meta_data_type (*dest, act->key); if (value_type == 0) /* not found */ continue; if (value_type != MD_TYPE_STRING) { - ERROR ("Target `replace': Attempting replace on metadata key `%s', " + WARNING ("Target `replace': Attempting replace on metadata key `%s', " "which isn't a string.", act->key); continue; @@ -390,7 +388,10 @@ static int tr_meta_data_action_invoke ( /* {{{ */ STATIC_ARRAY_SIZE (matches), matches, /* flags = */ 0); if (status == REG_NOMATCH) + { + sfree (value); continue; + } else if (status != 0) { char errbuf[1024] = ""; @@ -398,53 +399,56 @@ static int tr_meta_data_action_invoke ( /* {{{ */ regerror (status, &act->re, errbuf, sizeof (errbuf)); ERROR ("Target `replace': Executing a regular expression failed: %s.", errbuf); + sfree (value); continue; } - if (act->replacement != NULL) + if (act->replacement == NULL) { - meta_data_t *result; + /* no replacement; delete the key */ + DEBUG ("target_replace plugin: tr_meta_data_action_invoke: " + "deleting `%s'", act->key); + meta_data_delete (*dest, act->key); + sfree (value); + continue; + } - subst_status = subst (temp, sizeof (temp), value, - (size_t) matches[0].rm_so, (size_t) matches[0].rm_eo, + subst_status = subst (temp, sizeof (temp), value, + (size_t) matches[0].rm_so, (size_t) matches[0].rm_eo, act->replacement); + if (subst_status == NULL) + { + ERROR ("Target `replace': subst (value = %s, start = %zu, end = %zu, " + "replacement = %s) failed.", + value, (size_t) matches[0].rm_so, (size_t) matches[0].rm_eo, act->replacement); - if (subst_status == NULL) - { - ERROR ("Target `replace': subst (value = %s, start = %zu, end = %zu, " - "replacement = %s) failed.", - value, (size_t) matches[0].rm_so, (size_t) matches[0].rm_eo, - act->replacement); - continue; - } - - DEBUG ("target_replace plugin: tr_meta_data_action_invoke: `%s' " - "value `%s' -> `%s'", act->key, value, temp); - - if ((result = meta_data_create()) == NULL) - { - ERROR ("Target `replace': failed to create metadata for `%s'.", - act->key); - return (-ENOMEM); - } - - meta_data_status = meta_data_add_string (result, act->key, temp); - - if (meta_data_status != 0) - { - ERROR ("Target `replace': Unable to set metadata value for `%s'.", - act->key); - return (meta_data_status); - } - - meta_data_clone_merge (dest, result); - meta_data_destroy (result); + sfree (value); + continue; } - else /* no replacement; delete the key */ + + DEBUG ("target_replace plugin: tr_meta_data_action_invoke: `%s' " + "value `%s' -> `%s'", act->key, value, temp); + + if ((result = meta_data_create()) == NULL) { - DEBUG ("target_replace plugin: tr_meta_data_action_invoke: " - "deleting `%s'", act->key); - meta_data_delete (*dest, act->key); + ERROR ("Target `replace': failed to create metadata for `%s'.", + act->key); + sfree (value); + return (-ENOMEM); } + + meta_data_status = meta_data_add_string (result, act->key, temp); + if (meta_data_status != 0) + { + ERROR ("Target `replace': Unable to set metadata value for `%s'.", + act->key); + meta_data_destroy (result); + sfree (value); + return (meta_data_status); + } + + meta_data_clone_merge (dest, result); + meta_data_destroy (result); + sfree (value); } /* for (act = act_head; act != NULL; act = act->next) */ return (0); -- 2.30.2