From: Junio C Hamano <gitster@pobox.com>
Date: Sat, 29 Sep 2012 05:43:30 GMT
Subject: Re: [PATCH 3/8] longest_ancestor_length(): use string_list_split()
Message-ID: <7vtxuhidtp.fsf@alter.siamese.dyndns.org>
In-Reply-To: <5066863E.1030005@alum.mit.edu>

Michael Haggerty <mhagger@alum.mit.edu> writes:

> On 09/28/2012 12:48 AM, Junio C Hamano wrote:
>> Michael Haggerty <mhagger@alum.mit.edu> writes:
>> 
>>> -	for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
>>> -		for (colon = ceil; *colon && *colon != PATH_SEP; colon++);
>>> -		len = colon - ceil;
>>> +	string_list_split(&prefixes, prefix_list, PATH_SEP, -1);
>>> +
>>> +	for (i = 0; i < prefixes.nr; i++) {
>>> +		const char *ceil = prefixes.items[i].string;
>>> +		int len = strlen(ceil);
>>> +
>> 
>> Much nicer than the yucky original ;-)
>
> If your winky-smiley implies irony, then I would like to object.

No irony.  The original was hard to read, especially with the for
loop that does in-line strchr() on a single line.  The updated one
is much easier to read.

> normalize_path_copy() can only shrink paths, not grow them.  So the
> length check on ceil guarantees that the result of normalize_path_copy()
> will fit in buf.

OK.

Thanks.