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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On May 23, 2014, at 13:05, Jeff King wrote:
On Thu, May 22, 2014 at 03:52:08PM -0700, Kyle J. McKay wrote:

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.

Yeah, sorry if it sounded like I was complaining about your review
elsewhere. I mostly found it amusing that I got two opposite-direction
reviews.

I didn't seem like complaining to me.  I also was amused.  :)

I agree that clarifying the situation in the commit message is best, and
I've done that in the version I just posted.

It looks great. And I suspect you're correct that a modern compiler would optimize the index-based version to be as efficient as the pointer arithmetic version.

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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]