config_parse: have `git_config_parse` own entry value and name

The function `git_config_parse` uses several callbacks to pass data
along to the caller as it parses the file. One design shortcoming here
is that strings passed to those callbacks are expected to be freed by
them, which is really confusing.

Fix the issue by changing memory ownership here. Instead of expecting
the `on_variable` callbacks to free memory for `git_config_parse`, just
do it inside of `git_config_parse`. While this obviously requires a bit
more memory allocation churn due to having to copy both name and value
at some places, this shouldn't be too much of a burden.

3 files changed, 17 insertions, 16 deletions

diff --git a/src/config_file.c b/src/config_file.c
index 90255c929..84f6dd153 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -1022,8 +1022,8 @@ static int parse_conditional_include(git_config_parser *reader,
 static int read_on_variable(
 	git_config_parser *reader,
 	const char *current_section,
-	char *var_name,
-	char *var_value,
+	const char *var_name,
+	const char *var_value,
 	const char *line,
 	size_t line_len,
 	void *data)
@@ -1031,24 +1031,26 @@ static int read_on_variable(
 	diskfile_parse_state *parse_data = (diskfile_parse_state *)data;
 	git_buf buf = GIT_BUF_INIT;
 	git_config_entry *entry;
+	const char *c;
 	int result = 0;
 
 	GIT_UNUSED(line);
 	GIT_UNUSED(line_len);
 
-	git__strtolower(var_name);
-	git_buf_printf(&buf, "%s.%s", current_section, var_name);
-	git__free(var_name);
+	git_buf_puts(&buf, current_section);
+	git_buf_putc(&buf, '.');
+	for (c = var_name; *c; c++)
+		git_buf_putc(&buf, git__tolower(*c));
 
 	if (git_buf_oom(&buf)) {
-		git__free(var_value);
+		git_buf_free(&buf);
 		return -1;
 	}
 
 	entry = git__calloc(1, sizeof(git_config_entry));
 	GITERR_CHECK_ALLOC(entry);
 	entry->name = git_buf_detach(&buf);
-	entry->value = var_value;
+	entry->value = var_value ? git__strdup(var_value) : NULL;
 	entry->level = parse_data->level;
 	entry->include_depth = parse_data->depth;
 
@@ -1065,7 +1067,6 @@ static int read_on_variable(
 		result = parse_conditional_include(reader, parse_data,
 						   entry->name, entry->value);
 
-
 	return result;
 }
 
@@ -1249,8 +1250,8 @@ static int write_on_section(
 static int write_on_variable(
 	git_config_parser *reader,
 	const char *current_section,
-	char *var_name,
-	char *var_value,
+	const char *var_name,
+	const char *var_value,
 	const char *line,
 	size_t line_len,
 	void *data)
@@ -1279,9 +1280,6 @@ static int write_on_variable(
 	if (has_matched && write_data->preg != NULL)
 		has_matched = (regexec(write_data->preg, var_value, 0, NULL, 0) == 0);
 
-	git__free(var_name);
-	git__free(var_value);
-
 	/* If this isn't the name/value we're looking for, simply dump the
 	 * existing data back out and continue on.
 	 */
diff --git a/src/config_parse.c b/src/config_parse.c
index 66ac43237..ffbc466e7 100644
--- a/src/config_parse.c
+++ b/src/config_parse.c
@@ -463,7 +463,7 @@ int git_config_parse(
 	void *data)
 {
 	git_parse_ctx *ctx;
-	char *current_section = NULL, *var_name, *var_value;
+	char *current_section = NULL, *var_name = NULL, *var_value = NULL;
 	int result = 0;
 
 	ctx = &parser->ctx;
@@ -508,7 +508,10 @@ int git_config_parse(
 		default: /* assume variable declaration */
 			if ((result = parse_variable(parser, &var_name, &var_value)) == 0 && on_variable) {
 				result = on_variable(parser, current_section, var_name, var_value, line_start, line_len, data);
+				git__free(var_name);
+				git__free(var_value);
 			}
+
 			break;
 		}
 
diff --git a/src/config_parse.h b/src/config_parse.h
index d14a8e60c..6650b87f3 100644
--- a/src/config_parse.h
+++ b/src/config_parse.h
@@ -36,8 +36,8 @@ typedef int (*git_config_parser_section_cb)(
 typedef int (*git_config_parser_variable_cb)(
 	git_config_parser *parser,
 	const char *current_section,
-	char *var_name,
-	char *var_value,
+	const char *var_name,
+	const char *var_value,
 	const char *line,
 	size_t line_len,
 	void *data);