On Mon, 13 Mar 2017 18:54:52 +0100
Nenad Merdanovic <nmer...@haproxy.com> wrote:
> Hey Willy,
>
> On 3/13/2017 6:32 PM, Willy Tarreau wrote:
> > Hi Nenad,
> >
> > [ccing Thierry]
> >
> > On Sun, Mar 12, 2017 at 10:00:51PM +0100, Nenad Merdanovic wrote:
> >> Signed-off-by: Nenad Merdanovic <nmer...@haproxy.com>
> >> ---
> >> include/proto/pattern.h | 2 --
> >> src/haproxy.c | 2 --
> >> src/pattern.c | 9 ++++++++-
> >> 3 files changed, 8 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/proto/pattern.h b/include/proto/pattern.h
> >> index 9c93db9..1a17445 100644
> >> --- a/include/proto/pattern.h
> >> +++ b/include/proto/pattern.h
> >> @@ -37,8 +37,6 @@ extern void (*pat_prune_fcts[PAT_MATCH_NUM])(struct
> >> pattern_expr *);
> >> extern struct pattern *(*pat_match_fcts[PAT_MATCH_NUM])(struct sample *,
> >> struct pattern_expr *, int);
> >> extern int pat_match_types[PAT_MATCH_NUM];
> >>
> >> -void pattern_finalize_config(void);
> >> -
> >> /* return the PAT_MATCH_* index for match name "name", or < 0 if not
> >> found */
> >> static inline int pat_find_match_name(const char *name)
> >> {
> >> diff --git a/src/haproxy.c b/src/haproxy.c
> >> index 559b481..bd7a5ca 100644
> >> --- a/src/haproxy.c
> >> +++ b/src/haproxy.c
> >> @@ -809,8 +809,6 @@ static void init(int argc, char **argv)
> >> exit(1);
> >> }
> >>
> >> - pattern_finalize_config();
> >> -
> >> err_code |= check_config_validity();
> >> if (err_code & (ERR_ABORT|ERR_FATAL)) {
> >> Alert("Fatal errors found in configuration.\n");
> >> diff --git a/src/pattern.c b/src/pattern.c
> >> index 60fe462..224e326 100644
> >> --- a/src/pattern.c
> >> +++ b/src/pattern.c
> >> @@ -2463,7 +2463,7 @@ int pattern_delete(struct pattern_expr *expr, struct
> >> pat_ref_elt *ref)
> >> /* This function finalize the configuration parsing. Its set all the
> >> * automatic ids
> >> */
> >> -void pattern_finalize_config(void)
> >> +static int pattern_finalize_config(void)
> >> {
> >> int i = 0;
> >> struct pat_ref *ref, *ref2, *ref3;
> >> @@ -2509,4 +2509,11 @@ void pattern_finalize_config(void)
> >> /* swap root */
> >> LIST_ADD(&pr, &pattern_reference);
> >> LIST_DEL(&pr);
> >> + return 0;
> >> +}
> >> +
> >> +__attribute__((constructor))
> >> +static void __pattern_init(void)
> >> +{
> >> + hap_register_post_check(pattern_finalize_config);
> >> }
> >
> > Are you sure about this one ? The post_check will cause the iniitialization
> > to be called *after* check_config_validity(). I'm not saying it's not
> > compatible, I just don't know if what it does is needed during the checks.
> > If you have already checked, I'm fine with it (but then please mention it
> > in the commit message).
>
> I checked and didn't see any impact to check_config_validity(). This
> function assigns unique IDs to the patterns for CLI purposes and
> initializes the pattern LRU cache. It is pretty much the same as
> tlskeys_finalize_config() which was already moved to post checks init.
>
> I'll resend the patch with the new commit message.
Hi, sorry for my response time. I read the code and I don't see any
suspicious point. In addition, we can take in account these points:
- The LRU are useless with the configuration check
- The pattern list id are not used by configuration, only by the cli,
so the configuration check connot fail if ID are wrongly positionned.
So, I think that this patch will not cause any problem. I think that
this initialization way is cleaner than the old function call in the
haproxy main code.
br,
Thierry.
> Regards,
> Nenad
>
> >
> > Thanks,
> > Willy
> >
