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

2014-05-23 Thread Jeff King
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

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

2014-05-23 Thread Kyle J. McKay
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

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

2014-05-22 Thread Junio C Hamano
Jeff King p...@peff.net writes: On Wed, May 21, 2014 at 05:07:36PM -0700, Kyle J. McKay wrote: +void strbuf_tolower(struct strbuf *sb) +{ + size_t i; + for (i = 0; i sb-len; i++) + sb-buf[i] = tolower(sb-buf[i]); +} + Wouldn't a direct transfer of the lowercase function

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

2014-05-22 Thread Jeff King
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

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

2014-05-22 Thread Junio C Hamano
Jeff King p...@peff.net writes: 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). ... ... I think the bigger question is: is this refactor worth doing, since there is only

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

2014-05-22 Thread Kyle J. McKay
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

[PATCH 2/9] strbuf: add strbuf_tolower function

2014-05-21 Thread Jeff King
This makes config's lowercase() function public. Note that we could continue to offer a pure-string lowercase, but there would be no callers (in most pure-string cases, we actually duplicate and lowercase the duplicate). Signed-off-by: Jeff King p...@peff.net --- This ones gets used later on...

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

2014-05-21 Thread Kyle J. McKay
On May 21, 2014, at 03:27, Jeff King wrote: This makes config's lowercase() function public. Note that we could continue to offer a pure-string lowercase, but there would be no callers (in most pure-string cases, we actually duplicate and lowercase the duplicate). Signed-off-by: Jeff King

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

2014-05-21 Thread Jeff King
On Wed, May 21, 2014 at 05:07:36PM -0700, Kyle J. McKay wrote: +void strbuf_tolower(struct strbuf *sb) +{ +size_t i; +for (i = 0; i sb-len; i++) +sb-buf[i] = tolower(sb-buf[i]); +} + Wouldn't a direct transfer of the lowercase function be something more like: