On 07/26/2011 03:32 PM, Lukas Fleischer wrote: > On Tue, Jul 26, 2011 at 01:18:14PM +0200, Ferry Huberts wrote: >> On 07/22/2011 05:15 PM, Lukas Fleischer wrote: >>> Some setenv() implementations (e.g. the one in OpenBSD's stdlib) >>> segfault if we pass a NULL value. Add an additional check to avoid this. >>> >>> Signed-off-by: Lukas Fleischer <[email protected]> >>> --- >>> shared.c | 2 +- >>> 1 files changed, 1 insertions(+), 1 deletions(-) >>> >>> diff --git a/shared.c b/shared.c >>> index 75c4b5c..0c8ce3e 100644 >>> --- a/shared.c >>> +++ b/shared.c >>> @@ -392,7 +392,7 @@ void cgit_prepare_repo_env(struct cgit_repo * repo) >>> p = env_vars; >>> q = p + env_var_count; >>> for (; p < q; p++) >>> - if (setenv(p->name, p->value, 1)) >>> + if (p->value && setenv(p->name, p->value, 1)) >>> fprintf(stderr, warn, p->name, p->value); >>> } >>> >> >> Lukas, >> >> I don't agree with this patch. >> >From cgitrc.5.txt, lines 501-503: >> >>> If a setting is not defined for a repository and the corresponding global >>> setting is also not defined (if applicable), then the corresponding >>> environment variable will be an empty string. >> >> So I'd like this differently, an empty string must be set. > > -1. Is there any reason to do this? Imho, we should leave undefined > variables unset and fix the documentation here. This is way more > straightforward and allows for distinguishing between unset and empty > values (this is cleaner, even though there might not be a use case yet).
Not true, I run this code on my servers. I agree with the fact that it is cleaner but am reluctant to have this change. I can live with that but let's ask Lars what he thinks about this -- Ferry Huberts _______________________________________________ cgit mailing list [email protected] http://hjemli.net/mailman/listinfo/cgit
