On May 22, 2014, at 06:42, Jeff King wrote:
[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.
I didn't see this patch before I sent the other email, but this is the
relevant part:
On May 22, 2014, at 15:52, Kyle J. McKay wrote:
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".
Or even just delete the "This makes config's lowercase() function
public" line after switching the implementation back.
--Kyle
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html