Re: [PATCH] include configuration directive for haproxy 1.5-dev3
On 14/12/2010 20:30, Brane F. Gračnar wrote: Hello :) Hi Brane, I haven't had time to test your patch, but after a visual review made some doubts to come up. And, please, post your patches inline, they are easier to review :) @@ -5343,6 +5440,40 @@ int readcfgfile(const char *file) err_code |= ERR_ALERT | ERR_FATAL; } [...] + memset(file_dir, '\0', buf_len); + strcpy(file_dir, file); + strcpy(file_dir, dirname(file_dir)); The first strcpy would be useless here? @@ -5193,6 +5195,99 @@ out: return err_code; } [...] + /** we want to support relative to include file glob patterns */ + int buf_len = 3; + if (dir != NULL) + buf_len += strlen(dir); Could you please clarify where does that magic 3 coming from?. As I said, I couldn't test the patch, and I can't figure it out for myself. Thanks a lot! -- L. Alberto Giménez
Re: [PATCH] include configuration directive for haproxy 1.5-dev3
On Wednesday 15 of December 2010 15:18:55 L. Alberto Giménez wrote: On 14/12/2010 20:30, Brane F. Gračnar wrote: Hello :) Hi Brane, I haven't had time to test your patch, but after a visual review made some doubts to come up. It also applies to 1.4.10. @@ -5343,6 +5440,40 @@ int readcfgfile(const char *file) err_code |= ERR_ALERT | ERR_FATAL; } [...] + memset(file_dir, '\0', buf_len); + strcpy(file_dir, file); + strcpy(file_dir, dirname(file_dir)); The first strcpy would be useless here? I experienced crashes without first strcpy. dirname(3) may change input buffer. From dirname(3): --- snip --- Both dirname() and basename() may modify the contents of path, so it may be desirable to pass a copy when calling one of these functions. --- snip --- @@ -5193,6 +5195,99 @@ out: return err_code; } [...] + /** we want to support relative to include file glob patterns */ + int buf_len = 3; + if (dir != NULL) + buf_len += strlen(dir); Could you please clarify where does that magic 3 coming from?. As I said, I couldn't test the patch, and I can't figure it out for myself. Initial buffer length should be 2 (To hold at least '/' and '\0' characters). I want to concatenate directory, /, glob pattern into variable real_pattern. Complete buffer length is: strlen(dir) + strlen(pattern) + 1 (slash character) + 1 (string termination byte). I set it to 3 becouse of just in case :) I think that this shouldn't be a problem becouse configuration parsing is done only at haproxy startup and real_pattern is always freed after usage. Best regards, Brane
Re: [PATCH] include configuration directive for haproxy 1.5-dev3
Hi Brane, Le mardi 14 décembre 2010 20:30:06, Brane F. Gračnar a écrit : Hello :) This patch (applies to 1.5-dev3) adds include configuration statement to haproxy configuration parser. I wrote this patch becouse my haproxy configuration became too big to be simply maintainable and becouse i really like conf.d configuration style. I don't know if you saw that but you can already have a conf.d configuration style in haproxy without any modification of code : haproxy accepts several - f parameters. As an example, find in attachment an init script we use on debian, based on the original script provided in the debian package with a few lines modified to recursively find .cfg files in the conf.d directory (maybe it's not the last version but here is the idea). -- Cyril Bonté haproxy.debian-multi.init Description: application/shellscript
