Re: [PATCHv4 1/6] environment: static list of repo-local env vars

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

On Wed, Feb 24, 2010 at 4:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> +/* Repository-local GIT_* environment variables */
>> +const char *const local_repo_env[] = {
>> +     ALTERNATE_DB_ENVIRONMENT,
>> +     CONFIG_ENVIRONMENT,
>> +     DB_ENVIRONMENT,
>> +     GIT_DIR_ENVIRONMENT,
>> +     GIT_WORK_TREE_ENVIRONMENT,
>> +     GRAFT_ENVIRONMENT,
>> +     INDEX_ENVIRONMENT,
>> +     NO_REPLACE_OBJECTS_ENVIRONMENT,
>> +     NULL
>> +};
>> +
>> +const unsigned int local_repo_env_size = ARRAY_SIZE(local_repo_env);
>
> This does not look very useful for two reasons.
>
>  - It counts the NULL at the tail, so the number is one-off; you cannot
>   say
>
>        for (i = 0; i < local_repo_env_size; i++)
>                do_something_to(local_repo_env[i]);

This is obviously very easy to fix by decrementing the size by 1 in
the definition.

>  - local_repo_env_size is not a compile time constant, so you cannot do:
>
>        const char *env[local_repo_env_size + 20];
>        memcpy(env, local_repo_env, sizeof(env[0]) * local_repo_env_size);
>        for (i = local_repo_env_size; i < ARRAY_SIZE(env); i++)
>                add_more_to(env + i, ...);

Indeed, I was just discussing this on the other thread. I personally
have no objection to using this C99 feature.

I guess #defining local_repo_env_size in cache.h, and keeping it
up-to-date when local_repo_env is updated is the best alternative.
This can be done in such a way that if it the array needs expansion,
forgetting to update its size is catched at compile time, but not for
contraction. Of course we don't expect that it will contract. The
disadvantage of this is that a change in cache.h requires an almost
complete recompile because almost everything depends on it. It's a
minor disadvantage, when compared to other approaches (double-walking
the list or other stuff).

(Of course, I still wonder what's wrong with C99 VLAs, but anyway).

-- 
Giuseppe "Oblomov" Bilotta