Re: [PATCH 1/1] clone: replace relative paths in the alternates

From: Hui Wang <jason77.wang@gmail.com>
Date: Aug 19, 2011, 06:39 UTC
Message-ID: <4E4E051B.5020803@gmail.com>
In-Reply-To: <7vy5yp53aj.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:

Show 15 quoted lines

> Hui Wang <jason77.wang@gmail.com> writes:
>
>   
>> +	mapsz = xsize_t(st.st_size);
>> +	map = xmmap(NULL, mapsz, PROT_READ, MAP_PRIVATE, fd, 0);
>> +	close(fd);
>> +
>> +	tmpbuf = mapsz > 4096 ? xmalloc(mapsz * 2) : xmalloc(mapsz + 4096);
>>     
>
> Where do these magic numbers come from, and what guarantee do we have that
> these magic numbers give safe upper bound for pathnames that are expanded
> to be absolute?
>
>

If an alternates file has dozens of relative path lines, this code has great risk.

Show 5 quoted lines

> Wouldn't it be a lot cleaner and much less error prone if you open the
> original, read from it with strbuf_readline(), convert the path using
> strbuf manipulation functions and write the resulting line out to the
> lockfile you obtained to update it, line by line?
>

This is a good suggestion, i will try to use the above way to prepare a V2 patch.

Thanks, Jason.