Re: [PATCH] git-mv: Fix error with multiple sources.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Jan 21, 2010, at 9:57 PM, Junio C Hamano wrote:

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

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]