Hi Ben,

On Wed, May 25, 2016 at 08:41:53AM +0100, Ben Cabot wrote:
> Sorry I forgot include the build details. The configuration its self
> does not seem to matter, you get the error if you if you load 2 empty
> files or 2 with any listen or frontend / backend configurations. Its
> just the fact you are loading 2 configuration files that causes the
> problem.

Thanks for reporting this. In fact it's interesting because this cleanup
patch has uncovered a real bug. Look at readcfgfile() in cfgparse.c, the
parsers are registered for each file. It just had the effect of wasting
memory and slightly slowing down the config parser as the number of files
increased, but now it fails. One more reason to keep it, and maybe even
to backport it in the end.

I've merged the attached patch to fix it.

Thanks,
Willy
>From 659fbf02300b721adef3de74a3c1a8e4d0851080 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Thu, 26 May 2016 17:55:28 +0200
Subject: BUG/MEDIUM: config: fix multiple declaration of section parsers

Ben Cabot reported that after commit 5e4261b ("CLEANUP: config:
detect double registration of a config section") recently introduced
in 1.7-dev, it's not possible anymore to load multiple configuration
files. Bryan Talbot provided a simple reproducer to exhibit the issue.

It turns out that function readcfgfile() registers new parsers for
section keywords for each new file. In addition to being useless, this
has the negative effect of wasting memory and slowing down the config
parser as the number of configuration files increases.

This fix only needs to be backported if/where the commit above is
backported.
---
 src/cfgparse.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 9b76465..fed5bd5 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -6968,19 +6968,6 @@ int readcfgfile(const char *file)
                return -1;
        }
 
-       /* Register internal sections */
-       if (!cfg_register_section("listen",   cfg_parse_listen) ||
-           !cfg_register_section("frontend", cfg_parse_listen) ||
-           !cfg_register_section("backend",  cfg_parse_listen) ||
-           !cfg_register_section("defaults", cfg_parse_listen) ||
-           !cfg_register_section("global",   cfg_parse_global) ||
-           !cfg_register_section("userlist", cfg_parse_users)  ||
-           !cfg_register_section("peers",    cfg_parse_peers)  ||
-           !cfg_register_section("mailers",  cfg_parse_mailers) ||
-           !cfg_register_section("namespace_list",    cfg_parse_netns) ||
-           !cfg_register_section("resolvers", cfg_parse_resolvers))
-               return -1;
-
        if ((f=fopen(file,"r")) == NULL) {
                free(thisline);
                return -1;
@@ -9132,6 +9119,22 @@ void cfg_unregister_sections(void)
        }
 }
 
+__attribute__((constructor))
+static void cfgparse_init(void)
+{
+       /* Register internal sections */
+       cfg_register_section("listen",         cfg_parse_listen);
+       cfg_register_section("frontend",       cfg_parse_listen);
+       cfg_register_section("backend",        cfg_parse_listen);
+       cfg_register_section("defaults",       cfg_parse_listen);
+       cfg_register_section("global",         cfg_parse_global);
+       cfg_register_section("userlist",       cfg_parse_users);
+       cfg_register_section("peers",          cfg_parse_peers);
+       cfg_register_section("mailers",        cfg_parse_mailers);
+       cfg_register_section("namespace_list", cfg_parse_netns);
+       cfg_register_section("resolvers",      cfg_parse_resolvers);
+}
+
 /*
  * Local variables:
  *  c-indent-level: 8
-- 
1.7.12.1

Reply via email to