On Wed, Feb 24, 2010 at 9:06 AM, Johannes Sixt <j.sixt@xxxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html