Re: [PATCH 2/2] strbuf: add strbuf_tolower function

> [re-adding list cc]
>
> On Thu, May 22, 2014 at 03:16:45PM +0200, Christian Couder wrote:
>
>>> +void strbuf_tolower(struct strbuf *sb)
>>> +{
>>> +       char *p;
>>> +       for (p = sb->buf; *p; p++)
>>> +               *p = tolower(*p);
>>> +}
>>
>> Last time I tried a change like the above, I was told that strbufs  
>> are
>> buffers that can contain NUL bytes. So maybe it should be:
>>
>>       for (p = sb->buf; p < sb->buf + sb->len; p++)
>>              *p = tolower(*p);
>
> Hah. I wrote it like that originally, and in review was asked to  
> switch
> it. I agree that it might be worth keeping strbuf functions 8bit- 
> clean.
> The original intent of the strbuf code was to make C strings easier to
> use, but I think we sometimes use them as general buffers, too.

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