From: Kyle J. McKay <mackyle@gmail.com>
Date: Thu, 22 May 2014 22:52:08 GMT
Subject: Re: [PATCH 2/9] strbuf: add strbuf_tolower function
Message-ID: <0E298290-243D-46E5-A177-C6D3553D7153@gmail.com>
In-Reply-To: <20140522184113.GC1167@sigill.intra.peff.net>

On May 22, 2014, at 11:41, Jeff King wrote:

> On Thu, May 22, 2014 at 11:36:37AM -0700, Junio C Hamano wrote:
>
>>> Yes, and that would be fine with me (I actually wrote  
>>> strbuf_tolower for
>>> my own use, and _then_ realized that we already had such a thing  
>>> that
>>> could be replaced).
>>
>> Do we forbid that sb->buf[x] for some x < sb->len to be NUL, and if
>> there is such a byte we stop running tolower() on the remainder?
>
> Christian brought this up elsewhere, and I agree it's probably  
> better to
> work over the whole buffer, NULs included. I'm happy to re-roll (or  
> you
> can just pick up the version of the patch in this thread),

The only reason I brought up the code difference in the first place  
was that the comment was "This makes config's lowercase() function  
public" which made me expect to see basically the equivalent of  
replacing a "static" with an "extern", but actually the function being  
made public was a different implementation than config's lowercase()  
function.  So it looks like the original PATCH 2/9 version of the code  
is best, but perhaps the comment can be tweaked a bit to not convey  
the same impression I got.  Maybe something like "Replace config's  
lowercase() function with a public one".

--Kyle