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

[Ferry Huberts]

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 <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).

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