On Wed, Feb 15, 2017 at 3:37 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: > Stefan Beller <sbeller@xxxxxxxxxx> writes: > >> Yes; though I'd place it in strbuf.{c,h} as it is operating >> on the internals of the strbuf. (Do we make any promises outside of >> strbuf about the internals? I mean we use .buf all the time, so maybe >> I am overly cautious here) > > I'd rather have it not use struct strbuf as an interface. It only > needs to pass "char *" and its promise that it touches the string > in-place without changing the length need to be documented as a > comment before the function. > >>> config.c | 16 +++++++++++++++- >>> 1 file changed, 15 insertions(+), 1 deletion(-) >>> >>> diff --git a/config.c b/config.c >>> index c6b874a7bf..98bf8fee32 100644 >>> --- a/config.c >>> +++ b/config.c >>> @@ -201,6 +201,20 @@ void git_config_push_parameter(const char *text) >>> strbuf_release(&env); >>> } >>> >>> +static void canonicalize_config_variable_name(struct strbuf *var) >>> +{ >>> + char *first_dot = strchr(var->buf, '.'); >>> + char *last_dot = strrchr(var->buf, '.'); >> >> If first_dot != NULL, then last_dot !+ NULL as well. >> (either both are NULL or none of them), >> so we can loose one condition below. > > I do not think it is worth it, though. > >>> + char *cp; >>> + >>> + if (first_dot) >>> + for (cp = var->buf; *cp && cp < first_dot; cp++) >>> + *cp = tolower(*cp); >>> + if (last_dot) >>> + for (cp = last_dot; *cp; cp++) >>> + *cp = tolower(*cp); > > if (first_dot) { > scan up to first dot > if (last_dot) just leave out the 'if (last_dot)' ? if (first_dot) { /* also implies last_dot */ do 0 -> first do last -> end }