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

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Feb 24, 2010, 10:07 UTC
Message-ID: <cb7bb73a1002240207u39176830scfa773b3e7d5c56@mail.gmail.com>
In-Reply-To: <4B84EEF2.2030502@viscovery.net>

On Wed, Feb 24, 2010 at 10:18 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:

Show 17 quoted lines

> 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