Merge pull request #4411 from pks-t/pks/config-parse-cleanups

Config parser cleanups

src/config_file.c

@@ -876,10 +876,8 @@ static char *escape_value(const char *ptr)
 		ptr++;
 	}
 
-	if (git_buf_oom(&buf)) {
-		git_buf_dispose(&buf);
+	if (git_buf_oom(&buf))
 		return NULL;
-	}
 
 	return git_buf_detach(&buf);
 }
@@ -1022,8 +1020,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 +1029,24 @@ 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);
+	if (git_buf_oom(&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 +1063,6 @@ static int read_on_variable(
 		result = parse_conditional_include(reader, parse_data,
 						   entry->name, entry->value);
 
-
 	return result;
 }
 
@@ -1249,8 +1246,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 +1276,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.
 	 */

src/config_parse.c

@@ -404,22 +404,21 @@ static int parse_name(
 static int parse_variable(git_config_parser *reader, char **var_name, char **var_value)
 {
 	const char *value_start = NULL;
-	char *line;
-	int quote_count;
+	char *line = NULL, *name = NULL, *value = NULL;
+	int quote_count, error;
 	bool multiline;
 
+	*var_name = NULL;
+	*var_value = NULL;
+
 	git_parse_advance_ws(&reader->ctx);
 	line = git__strndup(reader->ctx.line, reader->ctx.line_len);
-	if (line == NULL)
-		return -1;
+	GITERR_CHECK_ALLOC(line);
 
 	quote_count = strip_comments(line, 0);
 
-	/* If there is no value, boolean true is assumed */
-	*var_value = NULL;
-
-	if (parse_name(var_name, &value_start, reader, line) < 0)
-		goto on_error;
+	if ((error = parse_name(&name, &value_start, reader, line)) < 0)
+		goto out;
 
 	/*
 	 * Now, let's try to parse the value
@@ -428,30 +427,34 @@ static int parse_variable(git_config_parser *reader, char **var_name, char **var
 		while (git__isspace(value_start[0]))
 			value_start++;
 
-		if (unescape_line(var_value, &multiline, value_start, 0) < 0)
-			goto on_error;
+		if ((error = unescape_line(&value, &multiline, value_start, 0)) < 0)
+			goto out;
 
 		if (multiline) {
 			git_buf multi_value = GIT_BUF_INIT;
-			git_buf_attach(&multi_value, *var_value, 0);
+			git_buf_attach(&multi_value, value, 0);
 
 			if (parse_multiline_variable(reader, &multi_value, quote_count) < 0 ||
 			    git_buf_oom(&multi_value)) {
+				error = -1;
 				git_buf_dispose(&multi_value);
-				goto on_error;
+				goto out;
 			}
 
-			*var_value = git_buf_detach(&multi_value);
+			value = git_buf_detach(&multi_value);
 		}
 	}
 
-	git__free(line);
-	return 0;
+	*var_name = name;
+	*var_value = value;
+	name = NULL;
+	value = NULL;
 
-on_error:
-	git__free(*var_name);
+out:
+	git__free(name);
+	git__free(value);
 	git__free(line);
-	return -1;
+	return error;
 }
 
 int git_config_parse(
@@ -463,7 +466,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 +511,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;
 		}
 

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