Re: Graceful restart of Haproxy with SystemD

[Maxime de Roucy]

Le mercredi 08 juin 2016 à 21:21 +0200, Vincent Bernat a écrit :
> Just add ExecReload=/usr/sbin/haproxy -f /etc/haproxy/haproxy.cfg -c
> -q
> before the existing ExecReload.

Indeed:

[root@arch64-f5ff6f8ea5472b3f ~]# rm /tmp/*
rm: cannot remove '/tmp/*': No such file or directory
[root@arch64-f5ff6f8ea5472b3f ~]# systemctl cat test.service
# /etc/systemd/system/test.service
[Unit]
Description=test

[Service]
Type=simple
ExecStart=/usr/bin/sleep 3600
ExecReload=/usr/bin/touch /tmp/ExecReload1
ExecReload=/usr/bin/touch /tmp/ExecReload2
[root@arch64-f5ff6f8ea5472b3f ~]# systemctl start test.service
[root@arch64-f5ff6f8ea5472b3f ~]# systemctl reload test.service
[root@arch64-f5ff6f8ea5472b3f ~]# ls /tmp/
ExecReload1  ExecReload2

Also you will not be able to change the commande line argument (e.g.
adding a file with `-f …`) of the haproxy process with `systemctl
reload`.
But you can now load a directory with `-f`, add files in it and load
them (without service interruption) with a reload.
http://git.haproxy.org/?p=haproxy.git;a=commit;h=379d9c7c14e684ab1dcdb6467a6bf189153c2b1d

Regards
Maxime de Roucy

signature.asc
Description: This is a digitally signed message part

[PATCH] BUG/MEDIUM: init: don't use environment locale

[Maxime de Roucy]

This patch remove setlocale from the main function.

Some regex may have different behaviours depending on the
locale. Some LUA scripts may change their behaviour too
(http://lua-users.org/wiki/LuaLocales).

Without this patch (haproxy is using setlocale) :

$ cat locale.cfg
defaults
  mode http

frontend test
  bind :9000
  mode http
  use_backend testbk if { hdr_reg(X-Test) ^\w+$ }

backend testbk
  mode http
  server s 127.0.0.1:80

$ LANG=fr_FR.UTF-8 ./haproxy -f locale.cfg
$ curl -i -H "X-Test: échec" localhost:9000
HTTP/1.1 200 OK
...

$ LANG=C ./haproxy -f locale.cfg
$ curl -i -H "X-Test: échec" localhost:9000
HTTP/1.0 503 Service Unavailable
...
---
 doc/haproxy.1  | 4 ++--
 doc/management.txt | 2 +-
 src/haproxy.c  | 4 
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index cc9c702..73c2aee 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -35,8 +35,8 @@ instances without risking the system's stability.
 .TP
 \fB\-f \fP
 Specify configuration file or directory path. If the argument is a directory
-the files (and only files) it containes are added in lexical order (respecting
-LC_COLLATE) ; only non hidden files with ".cfg" extension are added.
+the files (and only files) it containes are added in lexical order (using
+LC_COLLATE=C) ; only non hidden files with ".cfg" extension are added.
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index 4f0af10..119c3fa 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -136,7 +136,7 @@ list of options is :
 
   -f  : adds  to the list of configuration files to be
 loaded. If  is a directory, all the files (and only files) it
-containes are added in lexical order (respecting LC_COLLATE) to the list of
+containes are added in lexical order (using LC_COLLATE=C) to the list of
 configuration files to be loaded ; only files with ".cfg" extension are
 added, only non hidden files (not prefixed with ".") are added.
 Configuration files are loaded and processed in their declaration order.
diff --git a/src/haproxy.c b/src/haproxy.c
index 582ad82..c6d1505 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -32,7 +32,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -1752,9 +1751,6 @@ int main(int argc, char **argv)
char errmsg[100];
int pidfd = -1;
 
-   /* get the locale from the environment variables */
-   setlocale(LC_ALL, "");
-
init(argc, argv);
signal_register_fct(SIGQUIT, dump, SIGQUIT);
signal_register_fct(SIGUSR1, sig_soft_stop, SIGUSR1);
-- 
2.8.2

[PATCH] BUG/MEDIUM: init: don't use environment locale

[Maxime de Roucy]

This patch remove setlocale from the main function.

Some regex may have different behaviours depending on the
locale. Some LUA scripts may change their behaviour too
(http://lua-users.org/wiki/LuaLocales).

Without this patch (haproxy is using setlocale) :

$ cat locale.cfg
defaults
  mode http

frontend test
  bind :9000
  mode http
  use_backend testbk if { hdr_reg(X-Test) ^\w+$ }

backend testbk
  mode http
  server s 127.0.0.1:80

$ LANG=fr_FR.UTF-8 ./haproxy -f locale.cfg
$ curl -i -H "X-Test: échec" localhost:9000
HTTP/1.1 200 OK
...

$ LANG=C ./haproxy -f locale.cfg
$ curl -i -H "X-Test: échec" localhost:9000
HTTP/1.0 503 Service Unavailable
...
---
 doc/haproxy.1  | 4 ++--
 doc/management.txt | 2 +-
 src/haproxy.c  | 4 
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index cc9c702..73c2aee 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -35,8 +35,8 @@ instances without risking the system's stability.
 .TP
 \fB\-f \fP
 Specify configuration file or directory path. If the argument is a directory
-the files (and only files) it containes are added in lexical order (respecting
-LC_COLLATE) ; only non hidden files with ".cfg" extension are added.
+the files (and only files) it containes are added in lexical order (using
+LC_COLLATE=C) ; only non hidden files with ".cfg" extension are added.
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index 4f0af10..119c3fa 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -136,7 +136,7 @@ list of options is :
 
   -f  : adds  to the list of configuration files to be
 loaded. If  is a directory, all the files (and only files) it
-containes are added in lexical order (respecting LC_COLLATE) to the list of
+containes are added in lexical order (using LC_COLLATE=C) to the list of
 configuration files to be loaded ; only files with ".cfg" extension are
 added, only non hidden files (not prefixed with ".") are added.
 Configuration files are loaded and processed in their declaration order.
diff --git a/src/haproxy.c b/src/haproxy.c
index 582ad82..c6d1505 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -32,7 +32,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -1752,9 +1751,6 @@ int main(int argc, char **argv)
char errmsg[100];
int pidfd = -1;
 
-   /* get the locale from the environment variables */
-   setlocale(LC_ALL, "");
-
init(argc, argv);
signal_register_fct(SIGQUIT, dump, SIGQUIT);
signal_register_fct(SIGUSR1, sig_soft_stop, SIGUSR1);
-- 
2.8.2

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[maxime de Roucy]

I can do it tomorow evening (France time).
Le 17 mai 2016 11:18 PM, "Willy Tarreau"  a écrit :

> On Tue, May 17, 2016 at 11:12:35PM +0200, Nenad Merdanovic wrote:
> > Hello Willy,
> >
> > On 05/17/2016 09:41 PM, Willy Tarreau wrote:
> > > Nenad, you were the one reporting the sorting issue, what do you think
> > > about all this ?
> >
> > I don't have strong feelings about this -- the initial point I asked
> > about was the versionsort vs alphasort and then just pointing out that
> > we've made a change in sorting with not following the locale.
> >
> > We can resolve this in the documentation as well.
>
> OK so if everyone feels the same, I guess it's better to remove the
> setlocale() all and adjust the doc then. Who wants to do it ? Please
> mention the side effects Cyril reported in the commit message so that
> we're not tempted to reinvent it later without a clue.
>
> Thanks!
> Willy
>
>

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Le mardi 17 mai 2016 à 21:28 +0200, Cyril Bonté a écrit :
> Well, I'd rather think we shouldn't bother with setlocale(), stay
> with the C locale, and document how the files are sorted. The
> behaviour is different than calling "haproxy -- *.cfg", where globing
> is made by the shell itself and its locale, but this looks to be
> consistent with how other programs behave (apache with the "Include"
> directive, nginx with the "include" one).

I agree. We should remove setlocale and document the fact that sorting
is done with LC_COLLATE=C.
-- 
Regards
Maxime

signature.asc
Description: This is a digitally signed message part

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hello Willy,

Le vendredi 13 mai 2016 à 19:22 +0200, Willy Tarreau a écrit :
> Maxime, could you please add a call to setlocale(LC_ALL, "") as
> suggested by Nenad before starting to scan the directories ? If
> something isn't 100% clear to you, do not hesitate to ask for more
> details, you won't look stupid, I did before you :-)

It's clear.
I didn't know either that I had to call setlocale to get the locale
from the environment. I read the man strcoll and thought it would use
LC_COLLATE (from the environment) without other call.

I add the setlocale call in the main function because it doesn't set
any haproxy variable. But "init" also sound like a good place.
Let me know if you prefer to move setlocale from main.

$ LC_COLLATE='fr_FR.utf8' ls -l rootdir/aaa
total 0
-rw-r--r-- 1 max users 0 13 mai   22:17 a.cfg
-rw-r--r-- 1 max users 0 13 mai   22:18 A.cfg
-rw-r--r-- 1 max users 0 13 mai   22:17 à.cfg
-rw-r--r-- 1 max users 0 13 mai   22:18 À.cfg
-rw-r--r-- 1 max users 0 13 mai   22:17 â.cfg
-rw-r--r-- 1 max users 0 13 mai   22:18 Â.cfg
$ LC_COLLATE='fr_FR.utf8' ./haproxy -C rootdir -f aaa 
aaa/a.cfg
aaa/A.cfg
aaa/à.cfg
aaa/À.cfg
aaa/â.cfg
aaa/Â.cfg
$ LC_COLLATE=C ./haproxy -C rootdir -f aaa 
aaa/A.cfg
aaa/a.cfg
aaa/À.cfg
aaa/Â.cfg
aaa/à.cfg
    aaa/â.cfg

-- 
Regards
Maxime de Roucy

signature.asc
Description: This is a digitally signed message part

[PATCH 1/3] MINOR: add list_append_word function

[Maxime de Roucy]

int list_append_word(struct list *li, const char *str, char **err)

Append a copy of string  (inside a wordlist) at the end of
the list .
The caller is responsible for freeing the  and  copy memory
area using free().

On failure : return 0 and  filled with an error message.
---
 include/common/standard.h |  8 
 src/standard.c| 32 
 2 files changed, 40 insertions(+)

diff --git a/include/common/standard.h b/include/common/standard.h
index cd2208c..f123f1a 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -1089,4 +1089,12 @@ static inline unsigned long long rdtsc()
 }
 #endif
 
+/* append a copy of string  (in a wordlist) at the end of the list 
+ * On failure : return 0 and  filled with an error message.
+ * The caller is responsible for freeing the  and  copy
+ * memory area using free()
+ */
+struct list;
+int list_append_word(struct list *li, const char *str, char **err);
+
 #endif /* _COMMON_STANDARD_H */
diff --git a/src/standard.c b/src/standard.c
index a4d2097..cfed94d 100644
--- a/src/standard.c
+++ b/src/standard.c
@@ -3439,6 +3439,38 @@ unsigned char utf8_next(const char *s, int len, unsigned 
int *c)
return code | ((p-(unsigned char *)s)&0x0f);
 }
 
+/* append a copy of string  (in a wordlist) at the end of the list 
+ * On failure : return 0 and  filled with an error message.
+ * The caller is responsible for freeing the  and  copy
+ * memory area using free()
+ */
+int list_append_word(struct list *li, const char *str, char **err)
+{
+   struct wordlist *wl;
+
+   wl = calloc(1, sizeof(*wl));
+   if (!wl) {
+   memprintf(err, "out of memory");
+   goto fail_wl;
+   }
+
+   wl->s = strdup(str);
+   if (!wl->s) {
+   memprintf(err, "out of memory");
+   goto fail_wl_s;
+   }
+
+   LIST_ADDQ(li, >list);
+
+   return 1;
+
+fail_wl_s:
+   free(wl->s);
+fail_wl:
+   free(wl);
+   return 0;
+}
+
 /*
  * Local variables:
  *  c-indent-level: 8
-- 
2.8.2

[PATCH 2/3] MEDIUM: init: use list_append_word in haproxy.c

[Maxime de Roucy]

replace LIST_ADDQ with list_append_word
---
 src/haproxy.c | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index bfd542c..3166bba 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -561,6 +561,7 @@ void init(int argc, char **argv)
char *tmp;
char *cfg_pidfile = NULL;
int err_code = 0;
+   char *err_msg = NULL;
struct wordlist *wl;
char *progname;
char *change_dir = NULL;
@@ -713,13 +714,12 @@ void init(int argc, char **argv)
/* now that's a cfgfile list */
argv++; argc--;
while (argc > 0) {
-   wl = calloc(1, sizeof(*wl));
-   if (!wl) {
-   Alert("Cannot load 
configuration file %s : out of memory.\n", *argv);
+   if (!list_append_word(_cfgfiles, 
*argv, _msg)) {
+   Alert("Cannot load 
configuration file/directory %s : %s\n",
+ *argv,
+ err_msg);
exit(1);
}
-   wl->s = *argv;
-   LIST_ADDQ(_cfgfiles, >list);
argv++; argc--;
}
break;
@@ -736,13 +736,12 @@ void init(int argc, char **argv)
case 'N' : cfg_maxpconn = atol(*argv); break;
case 'L' : strncpy(localpeer, *argv, 
sizeof(localpeer) - 1); break;
case 'f' :
-   wl = calloc(1, sizeof(*wl));
-   if (!wl) {
-   Alert("Cannot load 
configuration file %s : out of memory.\n", *argv);
+   if (!list_append_word(_cfgfiles, 
*argv, _msg)) {
+   Alert("Cannot load 
configuration file/directory %s : %s\n",
+ *argv,
+ err_msg);
exit(1);
}
-   wl->s = *argv;
-   LIST_ADDQ(_cfgfiles, >list);
break;
case 'p' : cfg_pidfile = *argv; break;
default: usage(progname);
@@ -1160,6 +1159,8 @@ void init(int argc, char **argv)
/* initialize structures for name resolution */
if (!dns_init_resolvers())
exit(1);
+
+   free(err_msg);
 }
 
 static void deinit_acl_cond(struct acl_cond *cond)
@@ -1550,6 +1551,7 @@ void deinit(void)
free(log);
}
list_for_each_entry_safe(wl, wlb, _cfgfiles, list) {
+   free(wl->s);
LIST_DEL(>list);
free(wl);
}
-- 
2.8.2

[PATCH 3/3] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

If -f argument is a directory add all the files (and only files) it
containes to the config files list.
These files are added in lexical order (respecting LC_COLLATE).
Only files with ".cfg" extension are added.
Only non hidden files (not prefixed with ".") are added.
Symlink are followed.
The -f order is still respected:

$ tree -a rootdir
rootdir
├── dir1
│   ├── 1.cfg
│   ├── 2
│   ├── 3.cfg
│   ├── 4.cfg -> 1.cfg
│   ├── 5 -> 1.cfg
│   ├── .6.cfg
│   ├── 7.cfg -> .
│   └── dir4
│   └── 8.cfg
├── dir2
│   ├── 10.cfg
│   └── 9.cfg
├── dir3
│   └── 11.cfg
├── link -> dir3/
├── root1
├── root2
└── root3

$ ./haproxy -C rootdir -f root2 -f dir2 -f root3 -f dir1 \
   -f link -f root1
root2
dir2/10.cfg
dir2/9.cfg
root3
dir1/1.cfg
dir1/3.cfg
dir1/4.cfg
link/11.cfg
root1

This can be useful on systemd where you can't change the haproxy
commande line options on service reload.
---
 doc/haproxy.1  |   8 ++--
 doc/management.txt |  44 +++--
 src/haproxy.c  | 110 +++--
 3 files changed, 135 insertions(+), 27 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index a836d5d..cc9c702 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -6,7 +6,7 @@ HAProxy \- fast and reliable http reverse proxy and load 
balancer
 
 .SH SYNOPSIS
 
-haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ maxconn] 
[\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] [\-dk] 
[\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
+haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ 
maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] 
[\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
 
 .SH DESCRIPTION
 
@@ -33,8 +33,10 @@ instances without risking the system's stability.
 .SH OPTIONS
 
 .TP
-\fB\-f \fP
-Specify configuration file path.
+\fB\-f \fP
+Specify configuration file or directory path. If the argument is a directory
+the files (and only files) it containes are added in lexical order (respecting
+LC_COLLATE) ; only non hidden files with ".cfg" extension are added.
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index e0469aa..4f0af10 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -124,26 +124,30 @@ enforce some settings without touching the configuration 
files. The current
 list of options is :
 
   -- * : all the arguments following "--" are paths to configuration
-file to be loaded and processed in the declaration order. It is mostly
-useful when relying on the shell to load many files that are numerically
-ordered. See also "-f". The difference between "--" and "-f" is that one
-"-f" must be placed before each file name, while a single "--" is needed
-before all file names. Both options can be used together, the command line
-ordering still applies. When more than one file is specified, each file
-must start on a section boundary, so the first keyword of each file must be
-one of "global", "defaults", "peers", "listen", "frontend", "backend", and
-so on. A file cannot contain just a server list for example.
-
-  -f  : adds  to the list of configuration files to be
-loaded. Configuration files are loaded and processed in their declaration
-order. This option may be specified multiple times to load multiple files.
-See also "--". The difference between "--" and "-f" is that one "-f" must
-be placed before each file name, while a single "--" is needed before all
-file names. Both options can be used together, the command line ordering
-still applies. When more than one file is specified, each file must start
-on a section boundary, so the first keyword of each file must be one of
-"global", "defaults", "peers", "listen", "frontend", "backend", and so
-on. A file cannot contain just a server list for example.
+file/directory to be loaded and processed in the declaration order. It is
+mostly useful when relying on the shell to load many files that are
+numerically ordered. See also "-f". The difference between "--" and "-f" is
+that one "-f" must be placed before each file name, while a single "--" is
+needed before all file names. Both options can be used together, the
+command line ordering still applies. When more than one file is specified,
+each file must start on a section boundary, so the first keyword of each
+file must be one of "global", "defaults", "peers", "listen", "frontend",
+"backend", and so on. A file cannot contain just a server list for example.
+
+  -f  : adds  to the list of configuration files to be
+loaded. If  

[PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

If -f argument is a directory add all the files (and only files) it
containes to the config files list.
These files are added in lexical order (man alphasort).
Only files with ".cfg" extension are added.
Only non hidden files (not prefixed with ".") are added.
Symlink are followed.
The -f order is still respected:

$ tree -a rootdir
rootdir
├── dir1
│   ├── 1.cfg
│   ├── 2
│   ├── 3.cfg
│   ├── 4.cfg -> 1.cfg
│   ├── 5 -> 1.cfg
│   ├── .6.cfg
│   ├── 7.cfg -> .
│   └── dir4
│   └── 8.cfg
├── dir2
│   ├── 10.cfg
│   └── 9.cfg
├── dir3
│   └── 11.cfg
├── link -> dir3/
├── root1
├── root2
└── root3

$ ./haproxy -C rootdir -f root2 -f dir2 -f root3 -f dir1 \
   -f link -f root1
root2
dir2/10.cfg
dir2/9.cfg
root3
dir1/1.cfg
dir1/3.cfg
dir1/4.cfg
link/11.cfg
root1

This can be useful on systemd where you can't change the haproxy
commande line options on service reload.
---
 doc/haproxy.1 |   8 +--
 doc/management.txt|  44 
 include/common/standard.h |   8 +++
 src/haproxy.c | 128 +-
 src/standard.c|  32 
 5 files changed, 183 insertions(+), 37 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index a836d5d..08ea9df 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -6,7 +6,7 @@ HAProxy \- fast and reliable http reverse proxy and load 
balancer
 
 .SH SYNOPSIS
 
-haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ maxconn] 
[\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] [\-dk] 
[\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
+haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ 
maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] 
[\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
 
 .SH DESCRIPTION
 
@@ -33,8 +33,10 @@ instances without risking the system's stability.
 .SH OPTIONS
 
 .TP
-\fB\-f \fP
-Specify configuration file path.
+\fB\-f \fP
+Specify configuration file or directory path. If the argument is a directory
+the files (and only files) it containes are added in lexical order (man
+alphasort) ; only non hidden files with ".cfg" extension are added.
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index e0469aa..69b3c18 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -124,26 +124,30 @@ enforce some settings without touching the configuration 
files. The current
 list of options is :
 
   -- * : all the arguments following "--" are paths to configuration
-file to be loaded and processed in the declaration order. It is mostly
-useful when relying on the shell to load many files that are numerically
-ordered. See also "-f". The difference between "--" and "-f" is that one
-"-f" must be placed before each file name, while a single "--" is needed
-before all file names. Both options can be used together, the command line
-ordering still applies. When more than one file is specified, each file
-must start on a section boundary, so the first keyword of each file must be
-one of "global", "defaults", "peers", "listen", "frontend", "backend", and
-so on. A file cannot contain just a server list for example.
-
-  -f  : adds  to the list of configuration files to be
-loaded. Configuration files are loaded and processed in their declaration
-order. This option may be specified multiple times to load multiple files.
-See also "--". The difference between "--" and "-f" is that one "-f" must
-be placed before each file name, while a single "--" is needed before all
-file names. Both options can be used together, the command line ordering
-still applies. When more than one file is specified, each file must start
-on a section boundary, so the first keyword of each file must be one of
-"global", "defaults", "peers", "listen", "frontend", "backend", and so
-on. A file cannot contain just a server list for example.
+file/directory to be loaded and processed in the declaration order. It is
+mostly useful when relying on the shell to load many files that are
+numerically ordered. See also "-f". The difference between "--" and "-f" is
+that one "-f" must be placed before each file name, while a single "--" is
+needed before all file names. Both options can be used together, the
+command line ordering still applies. When more than one file is specified,
+each file must start on a section boundary, so the first keyword of each
+file must be one of "global", "defaults", "peers", "listen", "frontend",
+"backend", and so on. A file cannot contain just a server list for example.
+
+  -f 

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

;log);
>  fail_log:
> free(s->txn);
>  fail_txn:
> free(s->res);
>  fail_res:
> free(s->req);
>  fail_req:
> free(s);
>  fail_s:
> return NULL;
> 
> And a nice side effect is that when you look at the assembly code,
> it's much smaller and much more efficient.
> 
> > It doesn't reduce the number of lines and, as after every alert we
> > call exit, there is no need to clean anything.
> 
> As I explained above I agree on this but code correctness and code
> cleanness are two different things. We try to have a bit of
> modularity because we know that code moves and that it's better if it
> can move safely. For example I recently reimplemented all the stats
> dump to be able to dump *all* the information we have. It was
> something like 50 patches. I felt like this code had not changed for
> 10 years and that it would not change for the 10 next years. One week
> later, Thierry sent me a huge patch exploding all my functions
> and moving them around in order to call them from Lua, something I
> hadn't even imagined.

Wow, that's an explanation, thanks.

I tried to use it on the following patch.
Did I used it correctly ?

> I guess you'll find me boring or annoying initially, but
> over time once you see other people change your code and avoid a few
> traps, you'll find that such comments make sense :-)

You are not boring at all :-)
And so far you explained (with lot's of details) all the changed you
asked me to make, so that's absolutely fine by me.

> > @@ -1550,6 +1644,20 @@ void deinit(void)
> >     free(log);
> >     }
> >     list_for_each_entry_safe(wl, wlb, _cfgfiles, list) {
> > +   int argc_i = argc - 1;
> > +   char **argv_i = argv + 1;
> > +   int is_arg = 0;
> > +
> > +   while (argc_i > 0) {
> > +   if (*argv_i == wl->s)
> > +           is_arg = 1;
> > +   argv_i++;
> > +   argc_i--;
> > +   }
> > +
> > +   if (!is_arg)
> > +   free(wl->s);
> > +
> >     LIST_DEL(>list);
> >     free(wl);
> >     }
> 
> I could have missed something but I think that the only purpose of
> this part is to decide whether or not you're going to free wl->s, am
> I right ?
> If so, why not simply free(wl->s) unconditionally ? I think I may be
> missing something.

Some "wl->s" were pointing to "argv*" elements so we can't free theme.

In the following patch I decided to create a fonction
(list_append_word) that copy it's string argument en append it to a
list.
Because it's a copy (in the heap) we now have to free every "wl->s" ;
which also make things simpler.

Regards
Maxime de Roucy



diff --git a/include/common/standard.h b/include/common/standard.h
index cd2208c..f123f1a 100644
--- a/include/common/standard.h
+++ b/include/common/standard.h
@@ -1089,4 +1089,12 @@ static inline unsigned long long rdtsc()
 }
 #endif
 
+/* append a copy of string  (in a wordlist) at the end of the list 
+ * On failure : return 0 and  filled with an error message.
+ * The caller is responsible for freeing the  and  copy
+ * memory area using free()
+ */
+struct list;
+int list_append_word(struct list *li, const char *str, char **err);
+
 #endif /* _COMMON_STANDARD_H */
diff --git a/src/haproxy.c b/src/haproxy.c
index 0c223e5..6e5eecc 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -33,10 +33,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -423,7 +425,7 @@ void usage(char *name)
 {
display_version();
fprintf(stderr,
-   "Usage : %s [-f ]* [ -vdV"
+   "Usage : %s [-f <cfgfile|cfgdir>]* [ -vdV"
"D ] [ -n  ] [ -N  ]\n"
"[ -p  ] [ -m  ] [ -C  ] [-- 
*]\n"
"-v displays version ; -vv shows known build options.\n"
@@ -551,6 +553,87 @@ void dump(struct sig_handler *sh)
pool_gc2();
 }
 
+/* This function check if cfg_cfgfiles containes directories.
+ * If it find one, it add all the files (and only files) it containes
+ * in cfg_cfgfiles in place of the directory (and remove the directory).
+ * It add the files in lexical order.
+ */
+void cfgfiles_expand_directories(void)
+{
+   struct wordlist *wl, *wlb;
+   char *err = NULL;
+
+   list_for_each_entry_safe(wl, wlb, _cfgfiles, list) {
+   struct stat file_stat;
+   struct dirent **dir_entries;
+   int dir_entries_nb;
+   int dir_entries_it;
+
+   

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hello Willy,

I don't receive all the mails from haproxy@formilux.org.
For exemple I didn't received :
http://article.gmane.org/gmane.comp.web.haproxy/27795

However I am sure I sent a blank mail to haproxy+subscr...@formilux.org
 (I rechecked).
Can you check on the server side ?

> > I forgot to free the memory allocated at 'filename = calloc' (why
> > valgrind
> > didn't warn...). Forget this patch. I will send another one
> > tomorow.
> 
> Yes I noticed, and there's this one as well :
> 
> > > +   wlt = calloc(1, sizeof(*wlt));
> > > +   tmp_str = strdup(filename);
> > > +   if (!wlt || !tmp_str) {
> > > +   Alert("Cannot load
> > > configuration
> > > files %s : out of memory.\n",
> > > + filename);
> > > +   exit(1);
> > > +   }
> 
> If tmp_str fails and wlt succeeds, wlt is not freed.

If tmp_str fails and wlt succeeds we still got the Alert and everything
it freed on exit.
Anyway the problem isn't here anymore as I get ride of strdup.
See the end of this mail.

> Overall I still think that writing this into a dedicated function
> will make the controls and unrolling easier. I'd suggest something
> like this which is much more conventional, auditable and maintainable
> :
> 
> /* loads config file/dir , returns 0 on failure with errmsg
>  * filled with an error message.
>  */
> int load_config_file(const char *arg, char **errmsg)
> {
> ...
> wlt = calloc(1, sizeof(*wlt));
> if (!wlt) {
>  memprintf(errmsg, "out of memory");
>  goto fail_wlt;
> }
> 
> tmp_str = strdup(filename);
> if (!tmp_str) {
>  memprintf(errmsg, "out of memory");
>  goto fail_tmp_str;
> }
> ...
> return 1;
> ...
> fail_smp_str:
> free(wlt);
> fail_wlt:
> ...
> return 0;
> }
> 
> Then in init() :
> 
> if (!load_config_from_file(argv, )) {
> Alert("Cannot load configuration files %s : %s.\n", filename,
> errmsg);
> exit(1);
> }

I create the function "void cfgfiles_expand_directories(void)", but not
the "load_config_file" one.
I am not accustomed to using goto and it's hard for me to use it here
as I actually don't see the point of it (in
cfgfiles_expand_directories).

It doesn't reduce the number of lines and, as after every alert we call
exit, there is no need to clean anything.

> Also, please don't use sprintf() but snprintf() as I mentionned in
> the earlier mail.

I used sprintf at first because the man says it's not C89 compatible :
  fprintf(), printf(), sprintf(), vprintf(), vfprintf(), vsprintf(): 
POSIX.1-2001, POSIX.1-2008, C89, C99.
  snprintf(), vsnprintf(): POSIX.1-2001, POSIX.1-2008, C99.

and the CONTRIBUTING file says :
  It is important to keep in mind that certain facilities offered by
  recent versions must not be used in the code :
    - …
    - in general, anything which requires C99 (such as declaring
      variables in "for" statements)

I changed it to snprintf, yet I didn't check the output as it shouldn't
be able to fail (the string size == copied size == calloc size).

Here is a new version of the patch. Only for the src/haproxy.c file.
I will send a complete patch when everything will be settled.
I also free head memory from the "wlt = calloc(1, sizeof(*wlt));" line,
in the deinit function.
I don't think it's necessary as deinit is followed by exit but I
thought it would be cleaner as we also free cfg_cfgfiles elements.

Thank you very much for all your comments !
Maxime de Roucy

#

diff --git a/src/haproxy.c b/src/haproxy.c
index 0c223e5..3fae428 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -33,10 +33,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -423,7 +425,7 @@ void usage(char *name)
 {
display_version();
fprintf(stderr,
-   "Usage : %s [-f ]* [ -vdV"
+   "Usage : %s [-f <cfgfile|cfgdir>]* [ -vdV"
"D ] [ -n  ] [ -N  ]\n"
"[ -p  ] [ -m  ] [ -C  ] [-- 
*]\n"
"-v displays version ; -vv shows known build options.\n"
@@ -551,6 +553,95 @@ void dump(struct sig_handler *sh)
pool_gc2();
 }
 
+/* This function check if cfg_cfgfiles containes directories.
+ * If it find one, it add all th

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[maxime de Roucy]

I forgot to free the memory allocated at 'filename = calloc' (why valgrind
didn't warn...). Forget this patch. I will send another one tomorow.

Sorry
Le 9 mai 2016 11:28 PM, "Maxime de Roucy" <maxime.dero...@gmail.com> a
écrit :

> If -f argument is a directory add all the files (and only files) it
> containes to the config files list.
> These files are added in lexical order (man alphasort).
> The -f order is still respected:
>
> $ tree rootdir
> rootdir
> ├── root1
> ├── root2
> ├── root3
> ├── superdir1
> │   ├── aaa1
> │   ├── aaa2
> │   ├── aaa3
> │   ├── bbb1 -> aaa1
> │   └── wrong
> │   └── wrongfile
> └── superdir2
> ├── ccc1
> └── wronglink -> .
>
> $ ./haproxy -C rootdir -f root2 -f superdir2 \
>-f root3 -f superdir1 -f root1
> root2
> superdir2/ccc1
> root3
> superdir1/aaa1
> superdir1/aaa2
> superdir1/aaa3
> superdir1/bbb1
> root1
>
> This can be useful on systemd where you can't change the haproxy
> commande line options on service reload.
> ---
>  doc/haproxy.1  |   8 +++--
>  doc/management.txt |  23 ++--
>  src/haproxy.c  | 103
> ++---
>  3 files changed, 116 insertions(+), 18 deletions(-)
>
> diff --git a/doc/haproxy.1 b/doc/haproxy.1
> index a836d5d..6017efa 100644
> --- a/doc/haproxy.1
> +++ b/doc/haproxy.1
> @@ -6,7 +6,7 @@ HAProxy \- fast and reliable http reverse proxy and load
> balancer
>
>  .SH SYNOPSIS
>
> -haproxy \-f <configuration\ file> [\-L\ ] [\-n\ maxconn] [\-N\
> maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\
> ] [\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ]
> [{\-sf|\-st}\ pidlist...]
> +haproxy \-f <configuration\ file|dir> [\-L\ ] [\-n\ maxconn] [\-N\
> maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\
> ] [\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ]
> [{\-sf|\-st}\ pidlist...]
>
>  .SH DESCRIPTION
>
> @@ -33,8 +33,10 @@ instances without risking the system's stability.
>  .SH OPTIONS
>
>  .TP
> -\fB\-f \fP
> -Specify configuration file path.
> +\fB\-f \fP
> +Specify configuration file or directory path. If the argument is a
> directory
> +the files (and only files) it containes are added in lexical order (man
> +alphasort).
>
>  .TP
>  \fB\-L \fP
> diff --git a/doc/management.txt b/doc/management.txt
> index e0469aa..3e51d49 100644
> --- a/doc/management.txt
> +++ b/doc/management.txt
> @@ -134,16 +134,19 @@ list of options is :
>  one of "global", "defaults", "peers", "listen", "frontend",
> "backend", and
>  so on. A file cannot contain just a server list for example.
>
> -  -f  : adds  to the list of configuration files to be
> -loaded. Configuration files are loaded and processed in their
> declaration
> -order. This option may be specified multiple times to load multiple
> files.
> -See also "--". The difference between "--" and "-f" is that one "-f"
> must
> -be placed before each file name, while a single "--" is needed before
> all
> -file names. Both options can be used together, the command line
> ordering
> -still applies. When more than one file is specified, each file must
> start
> -on a section boundary, so the first keyword of each file must be one
> of
> -"global", "defaults", "peers", "listen", "frontend", "backend", and so
> -on. A file cannot contain just a server list for example.
> +  -f <cfgfile|cfgdir> : adds  to the list of configuration files
> to be
> +loaded. If  is a directory, all the files (and only files) it
> +containes are added in lexical order (man alphasort) to the list of
> +configuration files to be loaded. Configuration files are loaded and
> +processed in their declaration order. This option may be specified
> multiple
> +times to load multiple files. See also "--". The difference between
> "--"
> +and "-f" is that one "-f" must be placed before each file name, while
> a
> +single "--" is needed before all file names. Both options can be used
> +together, the command line ordering still applies. When more than one
> file
> +is specified, each file must start on a section boundary, so the first
> +keywor

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hi Willy,


> > Instead I create Alert_exit which embedded the exit call inside
> > Alert.
> > I just sent the patchs.
> > 
> > Is this solution good for you ?
> 
> To be honnest, no, I really don't like it. There are very few such
> locations that benefit from it and 2/3 of them even had the code
> ordering changed to be able to use the function (unrolling done
> before the message instead of after), which basically means that if
> any error or warning is printed in any of these functions (or if any
> crashes) the error output will be confusing or missing.

Indeed… let's forget about it.
-- 
Maxime de Roucy

signature.asc
Description: This is a digitally signed message part

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hi Willy and Cyril,

I just send a new version of the patch.
I made some changes following the remarks of Willy.

> > I'm not sure to like this feature in its current implementation.
> > I fear it will also create some new issues depending on how people
> > will use it.

Indeed it should be use with care.
But for me it's as dangerous as the '--' option as '--' had clearly
been implemented to be used with bash globling.
However with '--' the sysadmin can filter himself the files list ; it
can't with my patch.

> > For example, I know lots of sysadmin who have the (bad) habit to
> > make backup of the configuration files in the same directory,
> > without cleaning it up. We may see some directories like this :
> > 
> > service1.cfg
> > service2.cfg
> > service2.cfg~
> > service3.cfg.20160509
> > service4.cfg-19980101
> > service5.cfg
> > service5.cfg.old
> > service6.cfg.disabled
> > 
> > ...and so on.
> > 
> > When several sysadmins share the same haproxy instance, it can
> > quickly
> > become annoying.
> > 
> > Other use cases I immediately see :
> > - some configurations provide the crt files in the same directory,
> > which will break things
> > - some others will store map files in the same directory also
> > - what about configurations with a README or similar in the
> > directory ? or swap files because someone else is editing a file at
> > the same time ?

The last point is a very good one I think, swap files can easily be
forgotten.
The previous ones are more controversial for me as with my patch the
directory itself become a configuration "file". If sysadmin put "trash"
in it, it's the same as if he put "trash" in a configuration file… it's
not haproxy's fault if it fail.

However, I agree that we should implement some safeguards.

> I think that before going further we should start by trying to
> define what we want to see and what we don't want.
>  Maybe a solution could be to at least impose a file name extension
> (eg: .cfg), and I'm not sure it's enough. For example what should we
> do with symlinks, some will prefer to follow them, others not to.
> Most likely we should skip all dot files, etc.
> I think the discussion should go on before we go further with the
> code.

OK.

For me we should add a filter on file name ; keeping only ending with
".cfg" and not starting with ".".
I vote for following symlinks, as the implementation currently does.
It means less work for me :-) … Joking aside, I don't see any point
against it and sysadmins will assume it does except if it's explicitly
said in the docs (docs which I should complet regarding this point).
-- 
Regards
Maxime de Roucy

signature.asc
Description: This is a digitally signed message part

[PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

If -f argument is a directory add all the files (and only files) it
containes to the config files list.
These files are added in lexical order (man alphasort).
The -f order is still respected:

$ tree rootdir
rootdir
├── root1
├── root2
├── root3
├── superdir1
│   ├── aaa1
│   ├── aaa2
│   ├── aaa3
│   ├── bbb1 -> aaa1
│   └── wrong
│   └── wrongfile
└── superdir2
├── ccc1
└── wronglink -> .

$ ./haproxy -C rootdir -f root2 -f superdir2 \
   -f root3 -f superdir1 -f root1
root2
superdir2/ccc1
root3
superdir1/aaa1
superdir1/aaa2
superdir1/aaa3
superdir1/bbb1
root1

This can be useful on systemd where you can't change the haproxy
commande line options on service reload.
---
 doc/haproxy.1  |   8 +++--
 doc/management.txt |  23 ++--
 src/haproxy.c  | 103 ++---
 3 files changed, 116 insertions(+), 18 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index a836d5d..6017efa 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -6,7 +6,7 @@ HAProxy \- fast and reliable http reverse proxy and load 
balancer
 
 .SH SYNOPSIS
 
-haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ maxconn] 
[\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] [\-dk] 
[\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
+haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ 
maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] 
[\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
 
 .SH DESCRIPTION
 
@@ -33,8 +33,10 @@ instances without risking the system's stability.
 .SH OPTIONS
 
 .TP
-\fB\-f \fP
-Specify configuration file path.
+\fB\-f \fP
+Specify configuration file or directory path. If the argument is a directory
+the files (and only files) it containes are added in lexical order (man
+alphasort).
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index e0469aa..3e51d49 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -134,16 +134,19 @@ list of options is :
 one of "global", "defaults", "peers", "listen", "frontend", "backend", and
 so on. A file cannot contain just a server list for example.
 
-  -f  : adds  to the list of configuration files to be
-loaded. Configuration files are loaded and processed in their declaration
-order. This option may be specified multiple times to load multiple files.
-See also "--". The difference between "--" and "-f" is that one "-f" must
-be placed before each file name, while a single "--" is needed before all
-file names. Both options can be used together, the command line ordering
-still applies. When more than one file is specified, each file must start
-on a section boundary, so the first keyword of each file must be one of
-"global", "defaults", "peers", "listen", "frontend", "backend", and so
-on. A file cannot contain just a server list for example.
+  -f  : adds  to the list of configuration files to be
+loaded. If  is a directory, all the files (and only files) it
+containes are added in lexical order (man alphasort) to the list of
+configuration files to be loaded. Configuration files are loaded and
+processed in their declaration order. This option may be specified multiple
+times to load multiple files. See also "--". The difference between "--"
+and "-f" is that one "-f" must be placed before each file name, while a
+single "--" is needed before all file names. Both options can be used
+together, the command line ordering still applies. When more than one file
+is specified, each file must start on a section boundary, so the first
+keyword of each file must be one of "global", "defaults", "peers",
+"listen", "frontend", "backend", and so on. A file cannot contain just a
+server list for example.
 
   -C  : changes to directory  before loading configuration
 files. This is useful when using relative paths. Warning when using
diff --git a/src/haproxy.c b/src/haproxy.c
index 0c223e5..9824b7c 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -33,10 +33,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -423,7 +425,7 @@ void usage(char *name)
 {
display_version();
fprintf(stderr,
-   "Usage : %s [-f ]* [ -vdV"
+   "Usage : %s [-f ]* [ -vdV"
"D ] [ -n  ] [ -N  ]\n"
"[ -p  ] [ -m  ] [ -C  ] [-- 
*]\n"
"-v displays version ; -vv shows known build options.\n"
@@ -551,6 +553,94 @@ void dump(struct sig_handler *sh)
pool_gc2();
 }
 
+/* This function check if 

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hello,

Thanks for your remarks !

> I think this is a nice addition, it completes well the ability to
> load an arbitrary file list.

Good to hear that :)

> However I cannot merge it as is, there is a huge buffer overflow
> …

Ok, I see. I will try to patch that this evening.

> I have another comment here, please try to factor the error messages
> using a goto, this block appears at least 3 times :

I tried to use goto but there is always a slight difference in the
Alert(...) calls, so there is quite as many label as Alert call.
After some lines it seams error-prone and not very readable.

Instead I create Alert_exit which embedded the exit call inside Alert.
I just sent the patchs.

Is this solution good for you ?
-- 
Thanks
Maxime

signature.asc
Description: This is a digitally signed message part

[PATCH 2/2] MINOR: log: use Alert_exit in src/haproxy.c

[Maxime de Roucy]

This patch replace
  Alert(...); exit(1);
with
  Alert(1, ...)
when it's possible.
---
 src/haproxy.c | 176 +-
 1 file changed, 87 insertions(+), 89 deletions(-)

diff --git a/src/haproxy.c b/src/haproxy.c
index 0c223e5..c1acea6 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -698,10 +698,9 @@ void init(int argc, char **argv)
 
while (argc > 1 && argv[1][0] != '-') {
oldpids = realloc(oldpids, (nb_oldpids 
+ 1) * sizeof(int));
-   if (!oldpids) {
-   Alert("Cannot allocate old pid 
: out of memory.\n");
-   exit(1);
-   }
+   if (!oldpids)
+   Alert_exit(1, "Cannot allocate 
old pid : out of memory.\n");
+
argc--; argv++;
oldpids[nb_oldpids] = atol(*argv);
if (oldpids[nb_oldpids] <= 0)
@@ -714,10 +713,11 @@ void init(int argc, char **argv)
argv++; argc--;
while (argc > 0) {
wl = calloc(1, sizeof(*wl));
-   if (!wl) {
-   Alert("Cannot load 
configuration file %s : out of memory.\n", *argv);
-   exit(1);
-   }
+   if (!wl)
+   Alert_exit(1,
+  "Cannot load 
configuration file %s : out of memory.\n",
+  *argv);
+
wl->s = *argv;
LIST_ADDQ(_cfgfiles, >list);
argv++; argc--;
@@ -737,10 +737,11 @@ void init(int argc, char **argv)
case 'L' : strncpy(localpeer, *argv, 
sizeof(localpeer) - 1); break;
case 'f' :
wl = calloc(1, sizeof(*wl));
-   if (!wl) {
-   Alert("Cannot load 
configuration file %s : out of memory.\n", *argv);
-   exit(1);
-   }
+   if (!wl)
+   Alert_exit(1,
+  "Cannot load 
configuration file %s : out of memory.\n",
+  *argv);
+
wl->s = *argv;
LIST_ADDQ(_cfgfiles, >list);
break;
@@ -761,10 +762,11 @@ void init(int argc, char **argv)
if (LIST_ISEMPTY(_cfgfiles))
usage(progname);
 
-   if (change_dir && chdir(change_dir) < 0) {
-   Alert("Could not change to directory %s : %s\n", change_dir, 
strerror(errno));
-   exit(1);
-   }
+   if (change_dir && chdir(change_dir) < 0)
+   Alert_exit(1,
+  "Could not change to directory %s : %s\n",
+  change_dir,
+  strerror(errno));
 
global.maxsock = 10; /* reserve 10 fds ; will be incremented by socket 
eaters */
 
@@ -774,13 +776,15 @@ void init(int argc, char **argv)
int ret;
 
ret = readcfgfile(wl->s);
-   if (ret == -1) {
-   Alert("Could not open configuration file %s : %s\n",
- wl->s, strerror(errno));
-   exit(1);
-   }
+   if (ret == -1)
+   Alert_exit(1,
+  "Could not open configuration file %s : 
%s\n",
+  wl->s,
+  strerror(errno));
+
if (ret & (ERR_ABORT|ERR_FATAL))
Alert("Error(s) found in configuration file : %s\n", 
wl->s);
+
err_code |= ret;
if (err_code & ERR_ABORT)
exit(1);
@@ -792,10 +796,8 @@ void init(int argc, char **argv)
 #endif
 
err_code |= check_config_validity();
-   if (err_code & (ERR_ABORT|ERR_FATAL)) {
-   Alert("Fatal errors found in configuration.\n");
-   exit(1);
-   }
+   if (err_code & (ERR_ABORT|ERR_FATAL))
+   Alert_exit(1, "Fatal errors found in 

[PATCH 1/2] MINOR: log: Alert_exit function creation

[Maxime de Roucy]

Alert is often followed by exit.
Alert_exit embedded the exit call.
---
 include/proto/log.h |  9 +
 src/log.c   | 31 ++-
 2 files changed, 35 insertions(+), 5 deletions(-)

diff --git a/include/proto/log.h b/include/proto/log.h
index e606a3c..7afa2a0 100644
--- a/include/proto/log.h
+++ b/include/proto/log.h
@@ -90,8 +90,17 @@ void parse_logformat_string(const char *str, struct proxy 
*curproxy, struct list
  * Displays the message on stderr with the date and pid. Overrides the quiet
  * mode during startup.
  */
+void Alert_va(const char *fmt, va_list argp);
+/*
+ * Same as Alert_va with printf argument style
+ */
 void Alert(const char *fmt, ...)
__attribute__ ((format(printf, 1, 2)));
+/*
+ * Give the possibility to exit just after Alert.
+ */
+void Alert_exit(int exit_code, const char *fmt, ...)
+   __attribute__ ((format(printf, 2, 3)));
 
 /*
  * Displays the message on stderr with the date and pid.
diff --git a/src/log.c b/src/log.c
index 2d02247..949436a 100644
--- a/src/log.c
+++ b/src/log.c
@@ -622,23 +622,44 @@ void parse_logformat_string(const char *fmt, struct proxy 
*curproxy, struct list
  * Displays the message on stderr with the date and pid. Overrides the quiet
  * mode during startup.
  */
-void Alert(const char *fmt, ...)
+void Alert_va(const char *fmt, va_list argp)
 {
-   va_list argp;
struct tm tm;
 
if (!(global.mode & MODE_QUIET) || (global.mode & (MODE_VERBOSE | 
MODE_STARTING))) {
-   va_start(argp, fmt);
-
get_localtime(date.tv_sec, );
fprintf(stderr, "[ALERT] %03d/%02d%02d%02d (%d) : ",
tm.tm_yday, tm.tm_hour, tm.tm_min, tm.tm_sec, 
(int)getpid());
vfprintf(stderr, fmt, argp);
fflush(stderr);
-   va_end(argp);
}
 }
 
+/*
+ * Same as Alert_va with printf argument style
+ */
+void Alert(const char *fmt, ...)
+{
+   va_list argp;
+
+   va_start(argp, fmt);
+   Alert_va(fmt, argp);
+   va_end(argp);
+}
+
+/*
+ * Give the possibility to exit just after Alert.
+ */
+void Alert_exit(int exit_code, const char *fmt, ...)
+{
+   va_list argp;
+
+   va_start(argp, fmt);
+   Alert_va(fmt, argp);
+   va_end(argp);
+   if (exit_code)
+   exit(exit_code);
+}
 
 /*
  * Displays the message on stderr with the date and pid.
-- 
2.8.2

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hello,

The second version of the patch fix the typo already mentioned and
change :
- wlt->s = calloc(MAX_CLI_DIRFILE_NAME, 1);
+ wlt->s = calloc(strlen(filename) + 1, sizeof(*wlt->s));
-- 
Regards
Maxime de Roucy

signature.asc
Description: This is a digitally signed message part

[PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

If -f argument is a directory add all the files (and only files) it
containes to the config files list.
These files are added in lexical order (man alphasort).
The -f order is still respected:

$ tree rootdir
rootdir
├── root1
├── root2
├── root3
├── superdir1
│   ├── aaa1
│   ├── aaa2
│   ├── aaa3
│   ├── bbb1 -> aaa1
│   └── wrong
│   └── wrongfile
└── superdir2
├── ccc1
└── wronglink -> .

$ ./haproxy -C rootdir -f root2 -f superdir2 \
   -f root3 -f superdir1 -f root1
root2
superdir2/ccc1
root3
superdir1/aaa1
superdir1/aaa2
superdir1/aaa3
superdir1/bbb1
root1

This can be useful on systemd where you can't change the haproxy
commande line options on service reload.
---
 doc/haproxy.1 |  8 +++--
 doc/management.txt| 23 ++--
 include/common/defaults.h |  7 
 src/haproxy.c | 89 +++
 4 files changed, 108 insertions(+), 19 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index a836d5d..6017efa 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -6,7 +6,7 @@ HAProxy \- fast and reliable http reverse proxy and load 
balancer
 
 .SH SYNOPSIS
 
-haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ maxconn] 
[\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] [\-dk] 
[\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
+haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ 
maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] 
[\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
 
 .SH DESCRIPTION
 
@@ -33,8 +33,10 @@ instances without risking the system's stability.
 .SH OPTIONS
 
 .TP
-\fB\-f \fP
-Specify configuration file path.
+\fB\-f \fP
+Specify configuration file or directory path. If the argument is a directory
+the files (and only files) it containes are added in lexical order (man
+alphasort).
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index e0469aa..3e51d49 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -134,16 +134,19 @@ list of options is :
 one of "global", "defaults", "peers", "listen", "frontend", "backend", and
 so on. A file cannot contain just a server list for example.
 
-  -f  : adds  to the list of configuration files to be
-loaded. Configuration files are loaded and processed in their declaration
-order. This option may be specified multiple times to load multiple files.
-See also "--". The difference between "--" and "-f" is that one "-f" must
-be placed before each file name, while a single "--" is needed before all
-file names. Both options can be used together, the command line ordering
-still applies. When more than one file is specified, each file must start
-on a section boundary, so the first keyword of each file must be one of
-"global", "defaults", "peers", "listen", "frontend", "backend", and so
-on. A file cannot contain just a server list for example.
+  -f  : adds  to the list of configuration files to be
+loaded. If  is a directory, all the files (and only files) it
+containes are added in lexical order (man alphasort) to the list of
+configuration files to be loaded. Configuration files are loaded and
+processed in their declaration order. This option may be specified multiple
+times to load multiple files. See also "--". The difference between "--"
+and "-f" is that one "-f" must be placed before each file name, while a
+single "--" is needed before all file names. Both options can be used
+together, the command line ordering still applies. When more than one file
+is specified, each file must start on a section boundary, so the first
+keyword of each file must be one of "global", "defaults", "peers",
+"listen", "frontend", "backend", and so on. A file cannot contain just a
+server list for example.
 
   -C  : changes to directory  before loading configuration
 files. This is useful when using relative paths. Warning when using
diff --git a/include/common/defaults.h b/include/common/defaults.h
index 1c971d9..fefa30f 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -66,6 +66,13 @@
 #define MAX_SYSLOG_LEN  1024
 #endif
 
+/* maximum "directoryname/filename" string size when CLI
+ * -f argument is a directory (named "directoryname" here)
+ */
+#ifndef MAX_CLI_DIRFILE_NAME
+#define MAX_CLI_DIRFILE_NAME   1024
+#endif
+
 // maximum line size when parsing config
 #ifndef LINESIZE
 #define LINESIZE   2048
diff --git a/src/haproxy.c b/src/haproxy.c
index 0c223e5..29cd315 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -33,10 +33,12 @@
 #include 
 #include 
 #include 
+#include 
 

Re: [PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

Hello,

I just realize I include a typo when editing my commit message to
respect the 75 char limit :

>   $ ./haproxy -C rootdir -f root2 -f superdir2 \
>      -f root -f superdir1 -f root1

is in fact :

>   $ ./haproxy -C rootdir -f root2 -f superdir2 \
>      -f root3 -f superdir1 -f root1

Should I re-send my patch or is it OK ?
-- 
Regards
Maxime de Roucy

signature.asc
Description: This is a digitally signed message part

[PATCH] MEDIUM: init: allow directory as argument of -f

[Maxime de Roucy]

If -f argument is a directory add all the files (and only files) it
containes to the config files list.
These files are added in lexical order (man alphasort).
The -f order is still respected:

$ tree rootdir
rootdir
├── root1
├── root2
├── root3
├── superdir1
│   ├── aaa1
│   ├── aaa2
│   ├── aaa3
│   ├── bbb1 -> aaa1
│   └── wrong
│   └── wrongfile
└── superdir2
├── ccc1
└── wronglink -> .

$ ./haproxy -C rootdir -f root2 -f superdir2 \
   -f root -f superdir1 -f root1
root2
superdir2/ccc1
root3
superdir1/aaa1
superdir1/aaa2
superdir1/aaa3
superdir1/bbb1
root1

This can be useful on systemd where you can't change the haproxy
commande line options on service reload.
---
 doc/haproxy.1 |  8 +++--
 doc/management.txt| 23 ++--
 include/common/defaults.h |  7 
 src/haproxy.c | 89 +++
 4 files changed, 108 insertions(+), 19 deletions(-)

diff --git a/doc/haproxy.1 b/doc/haproxy.1
index a836d5d..6017efa 100644
--- a/doc/haproxy.1
+++ b/doc/haproxy.1
@@ -6,7 +6,7 @@ HAProxy \- fast and reliable http reverse proxy and load 
balancer
 
 .SH SYNOPSIS
 
-haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ maxconn] 
[\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] [\-dk] 
[\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
+haproxy \-f  [\-L\ ] [\-n\ maxconn] [\-N\ 
maxconn] [\-C\ ] [\-v|\-vv] [\-d] [\-D] [\-q] [\-V] [\-c] [\-p\ ] 
[\-dk] [\-ds] [\-de] [\-dp] [\-db] [\-dM[]] [\-m\ ] [{\-sf|\-st}\ 
pidlist...]
 
 .SH DESCRIPTION
 
@@ -33,8 +33,10 @@ instances without risking the system's stability.
 .SH OPTIONS
 
 .TP
-\fB\-f \fP
-Specify configuration file path.
+\fB\-f \fP
+Specify configuration file or directory path. If the argument is a directory
+the files (and only files) it containes are added in lexical order (man
+alphasort).
 
 .TP
 \fB\-L \fP
diff --git a/doc/management.txt b/doc/management.txt
index e0469aa..3e51d49 100644
--- a/doc/management.txt
+++ b/doc/management.txt
@@ -134,16 +134,19 @@ list of options is :
 one of "global", "defaults", "peers", "listen", "frontend", "backend", and
 so on. A file cannot contain just a server list for example.
 
-  -f  : adds  to the list of configuration files to be
-loaded. Configuration files are loaded and processed in their declaration
-order. This option may be specified multiple times to load multiple files.
-See also "--". The difference between "--" and "-f" is that one "-f" must
-be placed before each file name, while a single "--" is needed before all
-file names. Both options can be used together, the command line ordering
-still applies. When more than one file is specified, each file must start
-on a section boundary, so the first keyword of each file must be one of
-"global", "defaults", "peers", "listen", "frontend", "backend", and so
-on. A file cannot contain just a server list for example.
+  -f  : adds  to the list of configuration files to be
+loaded. If  is a directory, all the files (and only files) it
+containes are added in lexical order (man alphasort) to the list of
+configuration files to be loaded. Configuration files are loaded and
+processed in their declaration order. This option may be specified multiple
+times to load multiple files. See also "--". The difference between "--"
+and "-f" is that one "-f" must be placed before each file name, while a
+single "--" is needed before all file names. Both options can be used
+together, the command line ordering still applies. When more than one file
+is specified, each file must start on a section boundary, so the first
+keyword of each file must be one of "global", "defaults", "peers",
+"listen", "frontend", "backend", and so on. A file cannot contain just a
+server list for example.
 
   -C  : changes to directory  before loading configuration
 files. This is useful when using relative paths. Warning when using
diff --git a/include/common/defaults.h b/include/common/defaults.h
index 1c971d9..fefa30f 100644
--- a/include/common/defaults.h
+++ b/include/common/defaults.h
@@ -66,6 +66,13 @@
 #define MAX_SYSLOG_LEN  1024
 #endif
 
+/* maximum "directoryname/filename" string size when CLI
+ * -f argument is a directory (named "directoryname" here)
+ */
+#ifndef MAX_CLI_DIRFILE_NAME
+#define MAX_CLI_DIRFILE_NAME   1024
+#endif
+
 // maximum line size when parsing config
 #ifndef LINESIZE
 #define LINESIZE   2048
diff --git a/src/haproxy.c b/src/haproxy.c
index 0c223e5..69b1e8a 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -33,10 +33,12 @@
 #include 
 #include 
 #include 
+#include 
 #include