[PATCH 2/2] shared.c: Only setenv() if value is non-null

[Lukas Fleischer]

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 <cgit at cryptocrack.de>
> > ---
> >  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).

Objections?