From abe589303d4b26336d1159d2dbbafcc8446714ef Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Mon, 23 Apr 2018 21:42:42 -0400 Subject: [PATCH] Use TR_LIST for TR_FLINE's 'fspec' member * Replace custom iterators with generic iterator * Add 'steal' option to steal (or not) an item's talloc context when adding it to a TR_LIST * Add tr_list_foreach() function to iterate over a TR_LIST --- common/tr_config_filters.c | 29 ++++++------ common/tr_filter.c | 107 +++++++++++++++++---------------------------- common/tr_list.c | 40 ++++++++++++++--- include/tr_filter.h | 24 +++++----- include/tr_list.h | 8 +++- 5 files changed, 105 insertions(+), 103 deletions(-) diff --git a/common/tr_config_filters.c b/common/tr_config_filters.c index 063f812..2596af2 100644 --- a/common/tr_config_filters.c +++ b/common/tr_config_filters.c @@ -35,19 +35,10 @@ #include #include #include -#include #include #include -#include -#include -#include #include -#include -#include -#include -#include -#include #if JANSSON_VERSION_HEX < 0x020500 #include "jansson_iterators.h" @@ -146,17 +137,19 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR fline = tr_fline_new(tmp_ctx); if (fline == NULL) { - tr_debug("tr_cfg_parse_one_filter: Out of memory allocating filter line %d.", i + 1); + tr_debug("tr_cfg_parse_one_filter: Out of memory allocating filter line %d.", i); *rc = TR_CFG_NOMEM; goto cleanup; } - if (!strcmp(json_string_value(jfaction), "accept")) { fline->action = TR_FILTER_ACTION_ACCEPT; + tr_debug("tr_cfg_parse_one_filter: Filter action is 'accept'"); + } else if (!strcmp(json_string_value(jfaction), "reject")) { fline->action = TR_FILTER_ACTION_REJECT; + tr_debug("tr_cfg_parse_one_filter: Filter action is 'reject'"); } else { - tr_debug("tr_cfg_parse_one_filter: Error parsing filter action, unknown action' %s'.", + tr_debug("tr_cfg_parse_one_filter: Error parsing filter action, unknown action '%s'.", json_string_value(jfaction)); *rc = TR_CFG_NOPARSE; goto cleanup; @@ -202,6 +195,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR } /*For each filter spec within the filter line... */ + tr_debug("tr_cfg_parse_one_filter: Filter line has %d spec(s)", json_array_size(jfspecs)); json_array_foreach(jfspecs, j, this_jfspec) { if ((NULL == (jfield = json_object_get(this_jfspec, "field"))) || (!json_is_string(jfield))) { @@ -261,7 +255,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR tr_fspec_add_match(fspec, name); } } - if (!tr_filter_validate_spec_field(ftype, fspec)){ + if (!tr_filter_validate_spec_field(ftype, fspec)) { tr_debug("tr_cfg_parse_one_filter: Invalid filter field \"%.*s\" for %s filter, spec %d, filter %d.", fspec->field->len, fspec->field->buf, @@ -271,7 +265,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR goto cleanup; } - if(tr_fline_add_spec(fline, fspec) == NULL) { + if (tr_fline_add_spec(fline, fspec) == NULL) { tr_debug("tr_cfg_parse_one_filter: Unable to add spec %d to line %d of %s filter.", j, i, tr_filter_type_to_string(filt->type)); } @@ -283,6 +277,7 @@ static TR_FILTER *tr_cfg_parse_one_filter(TALLOC_CTX *mem_ctx, json_t *jfilt, TR *rc = TR_CFG_NOMEM; goto cleanup; } + tr_debug("tr_cfg_parse_one_filter: Added line %d to %s filter", i, tr_filter_type_to_string(filt->type)); } /* check that the filter is valid */ @@ -343,12 +338,16 @@ TR_FILTER_SET *tr_cfg_parse_filters(TALLOC_CTX *mem_ctx, json_t *jfilts, TR_CFG_ /* finally, parse the filter */ tr_debug("tr_cfg_parse_filters: Found %s filter.", filt_label); filt = tr_cfg_parse_one_filter(tmp_ctx, jfilt, filt_type, rc); - tr_filter_set_add(filt_set, filt); if (*rc != TR_CFG_SUCCESS) { tr_debug("tr_cfg_parse_filters: Error parsing %s filter.", filt_label); *rc = TR_CFG_NOPARSE; goto cleanup; } + if (tr_filter_set_add(filt_set, filt) != 0) { + tr_debug("tr_cfg_parse_filters: Error adding %s filter to filter set.", filt_label); + *rc = TR_CFG_NOPARSE; + goto cleanup; + } } *rc=TR_CFG_SUCCESS; diff --git a/common/tr_filter.c b/common/tr_filter.c index 224110e..aaa9b35 100644 --- a/common/tr_filter.c +++ b/common/tr_filter.c @@ -464,6 +464,17 @@ void tr_fspec_free(TR_FSPEC *fspec) talloc_free(fspec); } +/** + * Helper for tr_fspec_destructor - calls tr_free_name on its first argument + * + * @param item void pointer to a TR_NAME + * @param cookie ignored + */ +static void fspec_destruct_helper(void *item, void *cookie) +{ + TR_NAME *name = (TR_NAME *) item; + tr_free_name(name); +} static int tr_fspec_destructor(void *obj) { TR_FSPEC *fspec = talloc_get_type_abort(obj, TR_FSPEC); @@ -472,7 +483,7 @@ static int tr_fspec_destructor(void *obj) tr_free_name(fspec->field); if (fspec->match) - g_ptr_array_unref(fspec->match); + tr_list_foreach(fspec->match, fspec_destruct_helper, NULL); return 0; } @@ -483,7 +494,7 @@ TR_FSPEC *tr_fspec_new(TALLOC_CTX *mem_ctx) if (fspec != NULL) { fspec->field = NULL; - fspec->match = g_ptr_array_new_with_free_func((GDestroyNotify) tr_free_name); + fspec->match = tr_list_new(fspec); if (fspec->match == NULL) { talloc_free(fspec); return NULL; @@ -493,25 +504,25 @@ TR_FSPEC *tr_fspec_new(TALLOC_CTX *mem_ctx) return fspec; } -TR_NAME *tr_fspec_add_match(TR_FSPEC *fspec, TR_NAME *match) +/* Helper function and cookie structure for finding a match. The helper is called + * for every item in the match list, even after a match is found. If a match is found, + * match should be pointed to the matching item. If this is not NULL, do not change it + * because a match has already been found. */ +struct fspec_match_cookie { TR_NAME *name; TR_NAME *match;}; +static void fspec_match_helper(void *item, void *data) { - guint old_len = fspec->match->len; - g_ptr_array_add(fspec->match, match); - - if (fspec->match->len == old_len) - return NULL; /* failed to add */ - - return match; + TR_NAME *this_name = (TR_NAME *) item; + struct fspec_match_cookie *cookie = (struct fspec_match_cookie *) data; + if (cookie->match == NULL) { + if (tr_name_prefix_wildcard_match(cookie->name, this_name)) + cookie->match = this_name; + } } - /* returns 1 if the spec matches */ int tr_fspec_matches(TR_FSPEC *fspec, TR_FILTER_TYPE ftype, TR_FILTER_TARGET *target) { struct tr_filter_field_entry *field=NULL; - TR_NAME *name=NULL; - int retval=0; - - guint ii=0; + struct fspec_match_cookie cookie = {0}; if (fspec==NULL) return 0; @@ -525,31 +536,27 @@ int tr_fspec_matches(TR_FSPEC *fspec, TR_FILTER_TYPE ftype, TR_FILTER_TARGET *ta return 0; } - name=field->get(target); - if (name==NULL) + cookie.name = field->get(target); + if (cookie.name==NULL) return 0; /* if there's no value, there's no match */ - if (g_ptr_array_find_with_equal_func(fspec->match, - name, - (GEqualFunc) tr_name_prefix_wildcard_match, - &ii)) { - retval=1; + cookie.match = NULL; + tr_list_foreach(fspec->match, + fspec_match_helper, + &cookie); + if (cookie.match) { tr_debug("tr_fspec_matches: Field %.*s value \"%.*s\" matches \"%.*s\" for %s filter.", fspec->field->len, fspec->field->buf, - name->len, name->buf, - ((TR_NAME *)g_ptr_array_index(fspec->match,ii))->len, - ((TR_NAME *)g_ptr_array_index(fspec->match,ii))->buf, + cookie.name->len, cookie.name->buf, + cookie.match->len, cookie.match->buf, tr_filter_type_to_string(ftype)); - } - - if (!retval) { + } else { tr_debug("tr_fspec_matches: Field %.*s value \"%.*s\" does not match for %s filter.", fspec->field->len, fspec->field->buf, - name->len, name->buf, + cookie.name->len, cookie.name->buf, tr_filter_type_to_string(ftype)); } - tr_free_name(name); - return retval; + return (cookie.match != NULL); } void tr_fline_free(TR_FLINE *fline) @@ -641,7 +648,7 @@ void tr_fline_iter_free(TR_FLINE_ITER *iter) TR_FSPEC * tr_fline_iter_first(TR_FLINE_ITER *iter, TR_FLINE *fline) { - if (!iter || !fline) + if (!iter || !fline || !(fline->specs)) return NULL; iter->fline = fline; @@ -659,40 +666,6 @@ TR_FSPEC * tr_fline_iter_next(TR_FLINE_ITER *iter) return NULL; } -TR_FSPEC_ITER *tr_fspec_iter_new(TALLOC_CTX *mem_ctx) -{ - TR_FSPEC_ITER *iter = talloc(mem_ctx, TR_FSPEC_ITER); - if (iter) { - iter->fspec = NULL; - } - return iter; -} - -void tr_fspec_iter_free(TR_FSPEC_ITER *iter) -{ - talloc_free(iter); -} - -TR_NAME *tr_fspec_iter_first(TR_FSPEC_ITER *iter, TR_FSPEC *fspec) -{ - if (!iter || !fspec) - return NULL; - - iter->fspec = fspec; - iter->ii = 0; - return tr_fspec_iter_next(iter); -} - -TR_NAME *tr_fspec_iter_next(TR_FSPEC_ITER *iter) -{ - if (!iter) - return NULL; - - if (iter->ii < iter->fspec->match->len) - return g_ptr_array_index(iter->fspec->match, iter->ii++); - return NULL; -} - /** * Check that a filter is valid, i.e., can be processed. * @@ -746,7 +719,7 @@ int tr_filter_validate(TR_FILTER *filt) } /* check that at least one match is defined*/ - if (this_fspec->match->len == 0) { + if (tr_list_length(this_fspec->match) == 0) { talloc_free(tmp_ctx); return 0; } diff --git a/common/tr_list.c b/common/tr_list.c index 7d6c75c..2dbd07b 100644 --- a/common/tr_list.c +++ b/common/tr_list.c @@ -65,7 +65,20 @@ void tr_list_free(TR_LIST *list) talloc_free(list); } -void *tr_list_add(TR_LIST *list, void *item) +/** + * Add an item to the list + * + * If steal != 0, performs a talloc_steal() to put the new item in the + * list's context. If steal == 0, does not do this - in that case, you'll + * need to be sure that the memory is cleaned up through some other means. + * (This allows the list to represent non-talloc'ed items.) + * + * @param list list to add an item to + * @param item pointer to the item to add + * @param steal if non-zero, the item will be added to the list's context with talloc_steal() + * @return pointer to the added item or null if there was an error + */ +void *tr_list_add(TR_LIST *list, void *item, int steal) { guint old_len = (*list)->len; g_ptr_array_add((*list), item); @@ -73,9 +86,29 @@ void *tr_list_add(TR_LIST *list, void *item) if ((*list)->len == old_len) return NULL; /* failed to add */ + if (steal) + talloc_steal(list, item); + return item; } +size_t tr_list_length(TR_LIST *list) +{ + return (size_t) (*list)->len; +} + +/** + * Call func(item, cookie) on each item in the list. + * + * @param list list to iterate over + * @param func function, takes two void pointer arguments, first is the item, second the cookie + * @param cookie + */ +void tr_list_foreach(TR_LIST *list, TR_LIST_FOREACH_FUNC *func, void *cookie) +{ + g_ptr_array_foreach((*list), func, cookie); +} + TR_LIST_ITER *tr_list_iter_new(TALLOC_CTX *mem_ctx) { TR_LIST_ITER *iter = talloc(mem_ctx, TR_LIST_ITER); @@ -108,8 +141,3 @@ void *tr_list_iter_next(TR_LIST_ITER *iter) return g_ptr_array_index(*(iter->list), iter->index++); return NULL; } - -void tr_list_foreach(TR_LIST *list, void (*function)(void *, void *), void *cookie) -{ - g_ptr_array_foreach((*list), function, cookie); -} \ No newline at end of file diff --git a/include/tr_filter.h b/include/tr_filter.h index 0ca2f37..6fdbc5f 100644 --- a/include/tr_filter.h +++ b/include/tr_filter.h @@ -66,14 +66,9 @@ typedef enum { typedef struct tr_fspec { TR_NAME *field; - GPtrArray *match; + TR_LIST *match; } TR_FSPEC; -typedef struct tr_fspec_iter { - TR_FSPEC *fspec; - guint ii; -} TR_FSPEC_ITER; - typedef struct tr_fline { TR_FILTER_ACTION action; GPtrArray *specs; @@ -135,19 +130,22 @@ int tr_fspec_matches(TR_FSPEC *fspec, TR_FILTER_TYPE ftype, TR_FILTER_TARGET *ta typedef TR_LIST_ITER TR_FILTER_ITER; #define tr_filter_iter_new(CTX) (tr_list_iter_new(CTX)) #define tr_filter_iter_free(ITER) (tr_list_iter_free(ITER)) -#define tr_filter_iter_first(ITER, FILT) (tr_list_iter_first((ITER), (FILT)->lines)) -#define tr_filter_iter_next(ITER) (tr_list_iter_next(ITER)) -#define tr_filter_add_line(FILT, LINE) ((TR_FLINE *) tr_list_add((FILT)->lines, (LINE))) +#define tr_filter_iter_first(ITER, FILT) ((TR_FLINE *) tr_list_iter_first((ITER), (FILT)->lines)) +#define tr_filter_iter_next(ITER) ((TR_FLINE *) tr_list_iter_next(ITER)) +#define tr_filter_add_line(FILT, LINE) ((TR_FLINE *) tr_list_add((FILT)->lines, (LINE), 1)) TR_FLINE_ITER *tr_fline_iter_new(TALLOC_CTX *mem_ctx); void tr_fline_iter_free(TR_FLINE_ITER *iter); TR_FSPEC * tr_fline_iter_first(TR_FLINE_ITER *iter, TR_FLINE *fline); TR_FSPEC * tr_fline_iter_next(TR_FLINE_ITER *iter); -TR_FSPEC_ITER *tr_fspec_iter_new(TALLOC_CTX *mem_ctx); -void tr_fspec_iter_free(TR_FSPEC_ITER *iter); -TR_NAME *tr_fspec_iter_first(TR_FSPEC_ITER *iter, TR_FSPEC *fspec); -TR_NAME *tr_fspec_iter_next(TR_FSPEC_ITER *iter); +/* Iterator for TR_FSPEC matches */ +typedef TR_LIST_ITER TR_FSPEC_ITER; +#define tr_fspec_iter_new(CTX) (tr_list_iter_new(CTX)) +#define tr_fspec_iter_free(ITER) (tr_list_iter_free(ITER)) +#define tr_fspec_iter_first(ITER, SPEC) (tr_list_iter_first((ITER), (SPEC)->match)) +#define tr_fspec_iter_next(ITER) (tr_list_iter_next(ITER)) +#define tr_fspec_add_match(SPEC, MATCH) ((TR_NAME *) tr_list_add((SPEC)->match, (MATCH), 0)) /*In tr_constraint.c and exported, but not really a public symbol; needed by tr_filter.c and by tr_constraint.c*/ int TR_EXPORT tr_prefix_wildcard_match(const char *str, const char *wc_str); diff --git a/include/tr_list.h b/include/tr_list.h index 12ede45..2634cc6 100644 --- a/include/tr_list.h +++ b/include/tr_list.h @@ -40,19 +40,23 @@ typedef GPtrArray *TR_LIST; +typedef void (TR_LIST_FOREACH_FUNC)(void *item, void *cookie); + typedef struct tr_list_iter{ TR_LIST *list; guint index; } TR_LIST_ITER; TR_LIST *tr_list_new(TALLOC_CTX *mem_ctx); -void *tr_list_add(TR_LIST *list, void *item); void tr_list_free(TR_LIST *list); +void *tr_list_add(TR_LIST *list, void *item, int steal); +void *tr_list_index(TR_LIST *list, size_t index); +size_t tr_list_length(TR_LIST *list); TR_LIST_ITER *tr_list_iter_new(TALLOC_CTX *mem_ctx); void tr_list_iter_free(TR_LIST_ITER *iter); void *tr_list_iter_first(TR_LIST_ITER *iter, TR_LIST *list); void *tr_list_iter_next(TR_LIST_ITER *iter); -void tr_list_foreach(TR_LIST *list, void (*function)(void *item, void *cookie), void *cookie); +void tr_list_foreach(TR_LIST *list, TR_LIST_FOREACH_FUNC *func, void *cookie); #endif //TRUST_ROUTER_TR_LIST_H -- 2.1.4