Re: [PATCHv4 6/6] is_submodule_modified(): clear environment properly

[Giuseppe Bilotta <giuseppe.bilotta@gmail.com>]

On Wed, Feb 24, 2010 at 10:18 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Giuseppe Bilotta schrieb:
>> On Wed, Feb 24, 2010 at 9:06 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Giuseppe Bilotta schrieb:
>>>> +     const char *env[local_repo_env_size+2];
>>> Variable sized arrays are prohibited.
>>
>> Ah, sorry. Is alloca() allowed? I don't see it being used anywhere
>> else in the code, and malloc would be a little too much for this case.
>
> in cache.h:
>
> #define LOCAL_REPO_ENV_CAPACITY 20
> const char *const l_r_e[];
>
> in environtment.c:
>
> const char *const l_r_e[LOCAL_REPO_ENV_CAPACITY] = { ..., NULL };

What I don't like about this approach is that (1) it allocates more
memory than necessary and (2) it requires recompiling everything if
LOCAL_REPO_ENV_CAPACITY changes (since it's defined in cache.h which
is included basically everywhere. Admittedly (2) is _very_ unlikely to
happen if the #define is set large enough, but still ...

> and the compiler will croak if the constant becomes too small. (Oh, I
> know, you must use the 'inconsistent mode' that I nit-picked if you use
> this approach.)

(BTW, that approach requires slightly less index juggling and less C
code than a indexed for loop, so I kept it anyway, with a slightly
more expanded comment).


-- 
Giuseppe "Oblomov" Bilotta