Re: [PATCH] quote_path: fix collapsing of relative paths

[Jeff King <peff@peff.net>]

On Sun, Dec 02, 2007 at 09:56:54PM -0800, Junio C Hamano wrote:

> Somehow I feel it would be simpler and less error prone if you atually
> count to set len to the length if you got negative.
> [...]
> So perhaps this is easier to read?

Yes, I actually wrote the exact same patch but discarded it in favor of
what I sent, because I wanted a minimal change (and I assumed it had
been implemented that way to avoid the extra strlen call). But that is
just premature micro-optimization, and I think the version you posted is
much less subtle.

> The part that follows the patch, there is a line that subtracts a small
> number (off+1) from len --- while it won't have a wraparound issue, it
> feels a bit ugly to rely on the "magic -1" value to stay "magic
> negative" if small positive integers are subtracted from it.

I hadn't considered that, but another good reason to just set len in the
first place.

> Also the reason the updated condition to the while loop does not have to
> check NUL termination upon negative len is because both (prefix[off] !=
> NUL) and (prefix[off] == in[off]) are checked there, which some may find
> subtle.

Yes, I found it subtle, too, which is why I documented it in the commit
message. :)

> -	if (len > 0)
> -		strbuf_grow(out, len);
> -	strbuf_setlen(out, 0);
> +	if (len < 0)
> +		len = strlen(in);
>  
> +	strbuf_grow(out, len);
> +	strbuf_setlen(out, 0);

I wonder if this 'grow' is really all that useful, since we are now
going to overallocate in some cases. Perhaps we should just let strbuf
do its job and grow as necessary. But it likely doesn't matter either
way.

-Peff