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


Reply via email to