Re: [PATCH] git-mv: Fix error with multiple sources.
On Jan 21, 2010, at 9:57 PM, Junio C Hamano wrote:
Show 20 quoted lines
>> diff --git a/setup.c b/setup.c
>> index 710e2f3..80cf535 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -132,8 +132,8 @@ const char **get_pathspec(const char *prefix, const char **pathspec)
>> return NULL;
>>
>> if (!entry) {
>> - static const char *spec[2];
>> - spec[0] = prefix;
>> + const char **spec = xmalloc(sizeof(char *) * 2);
>> + spec[0] = xstrdup(prefix);
>> spec[1] = NULL;
>> return spec;
>> }
>
> I don't understand this change. Because elements of returned pathspec
> from this function are often simply the pathspec argument itself (which in
> turn is often argv[] of the calling program), and other times allocated by
> this function, the callers are never going to free() them.The xstrdup of prefix is for good measure. The important change is the removal of the static spec-array. Two invocations of get_pathspec with different prefixes could invalidate the contents of a pathspec in use.
When called with a non-empty pathspec, all the entries are allocated so I think it is reasonable to allocate them in the degenerate case as well. My impression of the git-code is that it is leaking quite a bit when it comes to strings anyway.