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

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

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.

>>       struct strbuf buf = STRBUF_INIT;
>>
>> +     /* Copy local_repo_env to env, letting i
>> +        rest at the last NULL */
>> +     while (env[i] = local_repo_env[i])
>> +             ++i; /* do nothing */
>> +
>
> This looks very inconsistent: At the one hand, you use l_r_e_size to
> allocate the space, but then you iterate over it assuming that the list is
> (also) NULL-terminated. But this is only a minor nit.

Well, if you consider that using l-r-e-size is just a way to spare a
double-walk, the inconsistency is tolerable. OTOH, in this particular
case NULL-walking the list doesn't give the usual benefit of sparing a
counter, so I could rework the patch to use a standard for loop. (I
also notice that I forgot to remove the /* do nothing */ comment from
the while(env[i] = local_repo_env[i++]) ; --i approach I was going
with at first)

-- 
Giuseppe "Oblomov" Bilotta