From f1c0892aa6ee28bdaf86ee787059c3add8d7eaec Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 2 Nov 2020 19:53:16 +0100 Subject: [PATCH] MINOR: pattern: remerge the list and tree deletion functions pat_del_tree_gen() was already chained onto pat_del_list_gen() to deal with remaining cases, so let's complete the merge and have a generic pattern deletion function acting on the reference and taking care of reliably removing all elements. --- include/haproxy/pattern.h | 5 ++- src/http_acl.c | 4 +-- src/pattern.c | 71 ++++++++++++++++++--------------------- 3 files changed, 36 insertions(+), 44 deletions(-) diff --git a/include/haproxy/pattern.h b/include/haproxy/pattern.h index 0046a7f25..844bc9102 100644 --- a/include/haproxy/pattern.h +++ b/include/haproxy/pattern.h @@ -78,12 +78,11 @@ int pat_idx_tree_pfx(struct pattern_expr *expr, struct pattern *pat, char **err) /* * - * The following functions deletes all patterns related to reference pattern + * The following function deletes all patterns related to reference pattern * element in pattern refernce . * */ -void pat_del_list_gen(struct pat_ref *ref, struct pat_ref_elt *elt); -void pat_del_tree_gen(struct pat_ref *ref, struct pat_ref_elt *elt); +void pat_delete_gen(struct pat_ref *ref, struct pat_ref_elt *elt); /* * diff --git a/src/http_acl.c b/src/http_acl.c index d4a0e4650..d31592e97 100644 --- a/src/http_acl.c +++ b/src/http_acl.c @@ -121,8 +121,8 @@ static struct acl_kw_list acl_kws = {ILH, { * and match method are related to the corresponding fetch methods. This * is very particular ACL declaration mode. */ - { "http_auth_group", NULL, PAT_MATCH_STR, NULL, pat_idx_list_str, pat_del_list_gen, NULL, pat_match_auth }, - { "method", NULL, PAT_MATCH_STR, pat_parse_meth, pat_idx_list_str, pat_del_list_gen, NULL, pat_match_meth }, + { "http_auth_group", NULL, PAT_MATCH_STR, NULL, pat_idx_list_str, pat_delete_gen, NULL, pat_match_auth }, + { "method", NULL, PAT_MATCH_STR, pat_parse_meth, pat_idx_list_str, pat_delete_gen, NULL, pat_match_meth }, { "path", "path", PAT_MATCH_STR }, { "path_beg", "path", PAT_MATCH_BEG }, diff --git a/src/pattern.c b/src/pattern.c index 2908bd001..0a2032e6d 100644 --- a/src/pattern.c +++ b/src/pattern.c @@ -80,20 +80,20 @@ int (*pat_index_fcts[PAT_MATCH_NUM])(struct pattern_expr *, struct pattern *, ch }; void (*pat_delete_fcts[PAT_MATCH_NUM])(struct pat_ref *, struct pat_ref_elt *) = { - [PAT_MATCH_FOUND] = pat_del_list_gen, - [PAT_MATCH_BOOL] = pat_del_list_gen, - [PAT_MATCH_INT] = pat_del_list_gen, - [PAT_MATCH_IP] = pat_del_tree_gen, - [PAT_MATCH_BIN] = pat_del_list_gen, - [PAT_MATCH_LEN] = pat_del_list_gen, - [PAT_MATCH_STR] = pat_del_tree_gen, - [PAT_MATCH_BEG] = pat_del_tree_gen, - [PAT_MATCH_SUB] = pat_del_list_gen, - [PAT_MATCH_DIR] = pat_del_list_gen, - [PAT_MATCH_DOM] = pat_del_list_gen, - [PAT_MATCH_END] = pat_del_list_gen, - [PAT_MATCH_REG] = pat_del_list_gen, - [PAT_MATCH_REGM] = pat_del_list_gen, + [PAT_MATCH_FOUND] = pat_delete_gen, + [PAT_MATCH_BOOL] = pat_delete_gen, + [PAT_MATCH_INT] = pat_delete_gen, + [PAT_MATCH_IP] = pat_delete_gen, + [PAT_MATCH_BIN] = pat_delete_gen, + [PAT_MATCH_LEN] = pat_delete_gen, + [PAT_MATCH_STR] = pat_delete_gen, + [PAT_MATCH_BEG] = pat_delete_gen, + [PAT_MATCH_SUB] = pat_delete_gen, + [PAT_MATCH_DIR] = pat_delete_gen, + [PAT_MATCH_DOM] = pat_delete_gen, + [PAT_MATCH_END] = pat_delete_gen, + [PAT_MATCH_REG] = pat_delete_gen, + [PAT_MATCH_REGM] = pat_delete_gen, }; void (*pat_prune_fcts[PAT_MATCH_NUM])(struct pattern_expr *) = { @@ -1402,15 +1402,26 @@ int pat_idx_tree_pfx(struct pattern_expr *expr, struct pattern *pat, char **err) return 1; } -/* Deletes all list-based patterns from reference . Note that all of their +/* Deletes all patterns from reference . Note that all of their * expressions must be locked, and the pattern lock must be held as well. */ -void pat_del_list_gen(struct pat_ref *ref, struct pat_ref_elt *elt) +void pat_delete_gen(struct pat_ref *ref, struct pat_ref_elt *elt) { - struct pattern_list *pat; - struct pattern_list *safe; + struct pattern_tree *tree, *tree_bck; + struct pattern_list *pat, *pat_bck; - list_for_each_entry_safe(pat, safe, &elt->list_head, from_ref) { + /* delete all known tree nodes. They are all allocated inline */ + list_for_each_entry_safe(tree, tree_bck, &elt->tree_head, from_ref) { + BUG_ON(tree->ref != elt); + + ebmb_delete(&tree->node); + LIST_DEL(&tree->from_ref); + free(tree->data); + free(tree); + } + + /* delete all list nodes and free their pattern entries (str/reg) */ + list_for_each_entry_safe(pat, pat_bck, &elt->list_head, from_ref) { /* Check equality. */ BUG_ON(pat->pat.ref != elt); @@ -1424,29 +1435,11 @@ void pat_del_list_gen(struct pat_ref *ref, struct pat_ref_elt *elt) free(pat->pat.data); free(pat); } + + /* update revision number to refresh the cache */ ref->revision = rdtsc(); } -/* Deletes all tree-based patterns from reference . Note that all of their - * expressions must be locked, and the pattern lock must be held as well. - */ -void pat_del_tree_gen(struct pat_ref *ref, struct pat_ref_elt *elt) -{ - struct pattern_tree *tree, *tree_bck; - - list_for_each_entry_safe(tree, tree_bck, &elt->tree_head, from_ref) { - BUG_ON(tree->ref != elt); - - ebmb_delete(&tree->node); - LIST_DEL(&tree->from_ref); - free(tree->data); - free(tree); - } - - /* Also check the lists ofr immediate IPs or case-insensitive strings */ - pat_del_list_gen(ref, elt); -} - void pattern_init_expr(struct pattern_expr *expr) { LIST_INIT(&expr->patterns);