Subject: Re: [PATCH 3/3] config: store "git -c" variables using more robust format

From: Jeff King <[email-masked]>
Date: Wed, 9 Dec 2020 11:34:57 -0500

On Wed, Dec 09, 2020 at 11:20:26AM -0500, Jeff King wrote:

> One other side effect I just noticed is that we're very aggressive about
> trimming leading and trailing whitespace in the old-style format, but
> the new one will store values verbatim. IMHO that's better overall, but
> we might consider a preparatory patch to remove that trimming
> explicitly.

Actually, it looks like we just trim either side of the key. Which
is...weird. We've never generated any, and I wouldn't expect people to
write:

  git -c '  some.key = value'

And even if they did, then "value" would have extra whitespace. So I
don't think this is really changing anything important, though I'm still
tempted to do something like the patch below to clean up the reading
side (and as a bonus, it gets rid of a strbuf_split call, which is a
terrible and awkward interface).

diff --git a/config.c b/config.c
index 04029e45dc..ede33cf3d0 100644
--- a/config.c
+++ b/config.c
@@ -516,29 +516,21 @@ static int config_parse_pair(const char *key, const char *value,
 int git_config_parse_parameter(const char *text,
 			       config_fn_t fn, void *data)
 {
-	const char *value;
-	struct strbuf **pair;
+	char *to_free = NULL;
+	const char *key, *value;
 	int ret;
 
-	pair = strbuf_split_str(text, '=', 2);
-	if (!pair[0])
-		return error(_("bogus config parameter: %s"), text);
-
-	if (pair[0]->len && pair[0]->buf[pair[0]->len - 1] == '=') {
-		strbuf_setlen(pair[0], pair[0]->len - 1);
-		value = pair[1] ? pair[1]->buf : "";
+	value = strchr(text, '=');
+	if (value) {
+		key = to_free = xmemdupz(text, value - text);
+		value++;
 	} else {
-		value = NULL;
+		key = text;
 	}
 
-	strbuf_trim(pair[0]);
-	if (!pair[0]->len) {
-		strbuf_list_free(pair);
-		return error(_("bogus config parameter: %s"), text);
-	}
+	ret = config_parse_pair(key, value, fn, data);
 
-	ret = config_parse_pair(pair[0]->buf, value, fn, data);
-	strbuf_list_free(pair);
+	free(to_free);
 	return ret;
 }