Hi Godbach,
On Mon, Jun 08, 2015 at 09:32:10PM +0800, Godbach wrote:
> Hi Willy,
>
> Since all block rules has been move to the beginning of the http-request
> rules in check_config_validity() by the the following codes:
>
> /* move any "block" rules at the beginning of the
> http-request rules */
> if (!LIST_ISEMPTY(&curproxy->block_rules)) {
> /* insert block_rules into http_req_rules at the
> beginning */
> curproxy->block_rules.p->n =
> curproxy->http_req_rules.n;
> curproxy->http_req_rules.n->p =
> curproxy->block_rules.p;
> curproxy->block_rules.n->p =
> &curproxy->http_req_rules;
> curproxy->http_req_rules.n =
> curproxy->block_rules.n;
> LIST_INIT(&curproxy->block_rules);
> }
I didn't remember we did this :-)
> As a result, there is no need to clean blocking rules in deinit() as below:
>
> list_for_each_entry_safe(cond, condb, &p->block_rules, list)
> {
> LIST_DEL(&cond->list);
> prune_acl_cond(cond);
> free(cond);
> }
Indeed!
> In addition, there is also another issue. The type of the members listed
> in block_rules has become *struct http_req_rule*, not *struct acl_cond*
> in earlier versions, maybe there is also potential risk to clean
> block_rules in deinit().
>
> So in my opinion, just remove the codes will be OK as below:
>
> diff --git a/src/haproxy.c b/src/haproxy.c
> index 0dddd53..eac6f44 100644
> --- a/src/haproxy.c
> +++ b/src/haproxy.c
> @@ -1020,12 +1020,6 @@ void deinit(void)
> free(cwl);
> }
>
> - list_for_each_entry_safe(cond, condb, &p->block_rules, list)
> {
> - LIST_DEL(&cond->list);
> - prune_acl_cond(cond);
> - free(cond);
> - }
> -
That's OK for me.
> list_for_each_entry_safe(cond, condb, &p->mon_fail_cond,
> list) {
> LIST_DEL(&cond->list);
> prune_acl_cond(cond);
>
>
> I can send a patch later if there is no problem.
Yes, please feel free to do so, we'll backport it into 1.5 as well.
> BTW, I only checked this issue in 1.5 branch.
It must affect 1.6 as well in my opinion.
Thanks,
Willy