On Wed, Feb 24, 2010 at 9:06 AM, Johannes Sixt [off-list ref] wrote:

Giuseppe Bilotta schrieb:

quoted

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

quoted

      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