config: examine whole file when writing

Previously we would try to be clever when writing the configuration
file and try to stop parsing (and simply copy the rest of the old
file) when we either found the value we were trying to write,
or when we left the section that value was in, the assumption being
that there was no more work to do.

Regrettably, you can have another section with the same name later
in the file, and we must cope with that gracefully, thus we read the
whole file in order to write a new file.

Now, writing a file looks even more than reading.  Pull the config
parsing out into its own function that can be used by both reading
and writing the configuration.

2 files changed, 421 insertions, 396 deletions

diff --git a/src/config_file.c b/src/config_file.c
index 010c494eb..3c906e522 100644
--- a/src/config_file.c
+++ b/src/config_file.c
@@ -114,8 +114,7 @@ typedef struct {
 	diskfile_backend *snapshot_from;
 } diskfile_readonly_backend;
 
-static int config_parse(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth);
-static int parse_variable(struct reader *reader, char **var_name, char **var_value);
+static int config_read(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth);
 static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char *value);
 static char *escape_value(const char *ptr);
 
@@ -288,7 +287,7 @@ static int config_open(git_config_backend *cfg, git_config_level_t level)
 	if (res == GIT_ENOTFOUND)
 		return 0;
 
-	if (res < 0 || (res = config_parse(b->header.values->values, b, reader, level, 0)) < 0) {
+	if (res < 0 || (res = config_read(b->header.values->values, b, reader, level, 0)) < 0) {
 		refcounted_strmap_free(b->header.values);
 		b->header.values = NULL;
 	}
@@ -313,7 +312,7 @@ static int config__refresh(git_config_backend *cfg)
 	reader = git_array_get(b->readers, git_array_size(b->readers) - 1);
 	GITERR_CHECK_ALLOC(reader);
 
-	if ((error = config_parse(values->values, b, reader, b->level, 0)) < 0)
+	if ((error = config_read(values->values, b, reader, b->level, 0)) < 0)
 		goto out;
 
 	git_mutex_lock(&b->header.values_mutex);
@@ -1200,398 +1199,6 @@ static int included_path(git_buf *out, const char *dir, const char *path)
 	return git_path_join_unrooted(out, path, dir, NULL);
 }
 
-static int config_parse(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth)
-{
-	int c;
-	char *current_section = NULL;
-	char *var_name;
-	char *var_value;
-	cvar_t *var;
-	git_buf buf = GIT_BUF_INIT;
-	int result = 0;
-	uint32_t reader_idx;
-
-	if (depth >= MAX_INCLUDE_DEPTH) {
-		giterr_set(GITERR_CONFIG, "Maximum config include depth reached");
-		return -1;
-	}
-
-	reader_idx = git_array_size(cfg_file->readers) - 1;
-	/* Initialize the reading position */
-	reader->read_ptr = reader->buffer.ptr;
-	reader->eof = 0;
-
-	/* If the file is empty, there's nothing for us to do */
-	if (*reader->read_ptr == '\0')
-		return 0;
-
-	skip_bom(reader);
-
-	while (result == 0 && !reader->eof) {
-
-		c = reader_peek(reader, SKIP_WHITESPACE);
-
-		switch (c) {
-		case '\n': /* EOF when peeking, set EOF in the reader to exit the loop */
-			reader->eof = 1;
-			break;
-
-		case '[': /* section header, new section begins */
-			git__free(current_section);
-			current_section = NULL;
-			result = parse_section_header(reader, &current_section);
-			break;
-
-		case ';':
-		case '#':
-			reader_consume_line(reader);
-			break;
-
-		default: /* assume variable declaration */
-			result = parse_variable(reader, &var_name, &var_value);
-			if (result < 0)
-				break;
-
-			git__strtolower(var_name);
-			git_buf_printf(&buf, "%s.%s", current_section, var_name);
-			git__free(var_name);
-
-			if (git_buf_oom(&buf)) {
-				git__free(var_value);
-				return -1;
-			}
-
-			var = git__calloc(1, sizeof(cvar_t));
-			GITERR_CHECK_ALLOC(var);
-			var->entry = git__calloc(1, sizeof(git_config_entry));
-			GITERR_CHECK_ALLOC(var->entry);
-
-			var->entry->name = git_buf_detach(&buf);
-			var->entry->value = var_value;
-			var->entry->level = level;
-			var->included = !!depth;
-
-
-			if ((result = append_entry(values, var)) < 0)
-				break;
-			else
-				result = 0;
-
-			/* Add or append the new config option */
-			if (!git__strcmp(var->entry->name, "include.path")) {
-				struct reader *r;
-				git_buf path = GIT_BUF_INIT;
-				char *dir;
-				uint32_t index;
-
-				r = git_array_alloc(cfg_file->readers);
-				/* The reader may have been reallocated */
-				reader = git_array_get(cfg_file->readers, reader_idx);
-				memset(r, 0, sizeof(struct reader));
-				if ((result = git_path_dirname_r(&path, reader->file_path)) < 0)
-					break;
-
-				/* We need to know our index in the array, as the next config_parse call may realloc */
-				index = git_array_size(cfg_file->readers) - 1;
-				dir = git_buf_detach(&path);
-				result = included_path(&path, dir, var->entry->value);
-				git__free(dir);
-
-				if (result < 0)
-					break;
-
-				r->file_path = git_buf_detach(&path);
-				git_buf_init(&r->buffer, 0);
-				result = git_futils_readbuffer_updated(&r->buffer, r->file_path, &r->file_mtime,
-									    &r->file_size, NULL);
-
-				if (result == 0) {
-					result = config_parse(values, cfg_file, r, level, depth+1);
-					r = git_array_get(cfg_file->readers, index);
-					reader = git_array_get(cfg_file->readers, reader_idx);
-				}
-				else if (result == GIT_ENOTFOUND) {
-					giterr_clear();
-					result = 0;
-				}
-
-				git_buf_free(&r->buffer);
-
-				if (result < 0)
-					break;
-			}
-
-			break;
-		}
-	}
-
-	git__free(current_section);
-	return result;
-}
-
-static int write_section(git_filebuf *file, const char *key)
-{
-	int result;
-	const char *dot;
-	git_buf buf = GIT_BUF_INIT;
-
-	/* All of this just for [section "subsection"] */
-	dot = strchr(key, '.');
-	git_buf_putc(&buf, '[');
-	if (dot == NULL) {
-		git_buf_puts(&buf, key);
-	} else {
-		char *escaped;
-		git_buf_put(&buf, key, dot - key);
-		escaped = escape_value(dot + 1);
-		GITERR_CHECK_ALLOC(escaped);
-		git_buf_printf(&buf, " \"%s\"", escaped);
-		git__free(escaped);
-	}
-	git_buf_puts(&buf, "]\n");
-
-	if (git_buf_oom(&buf))
-		return -1;
-
-	result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size);
-	git_buf_free(&buf);
-
-	return result;
-}
-
-static const char *quotes_for_value(const char *value)
-{
-	const char *ptr;
-
-	if (value[0] == ' ' || value[0] == '\0')
-		return "\"";
-
-	for (ptr = value; *ptr; ++ptr) {
-		if (*ptr == ';' || *ptr == '#')
-			return "\"";
-	}
-
-	if (ptr[-1] == ' ')
-		return "\"";
-
-	return "";
-}
-
-/*
- * This is pretty much the parsing, except we write out anything we don't have
- */
-static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char* value)
-{
-	int result, c;
-	int section_matches = 0, last_section_matched = 0, preg_replaced = 0, write_trailer = 0;
-	const char *pre_end = NULL, *post_start = NULL, *data_start, *write_start;
-	char *current_section = NULL, *section, *name, *ldot;
-	git_filebuf file = GIT_FILEBUF_INIT;
-	struct reader *reader = git_array_get(cfg->readers, 0);
-
-	/* We need to read in our own config file */
-	result = git_futils_readbuffer(&reader->buffer, cfg->file_path);
-
-	/* Initialise the reading position */
-	if (result == GIT_ENOTFOUND) {
-		reader->read_ptr = NULL;
-		reader->eof = 1;
-		data_start = NULL;
-		git_buf_clear(&reader->buffer);
-	} else if (result == 0) {
-		reader->read_ptr = reader->buffer.ptr;
-		reader->eof = 0;
-		data_start = reader->read_ptr;
-	} else {
-		return -1; /* OS error when reading the file */
-	}
-
-	write_start = data_start;
-
-	/* Lock the file */
-	if ((result = git_filebuf_open(
-		&file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
-			git_buf_free(&reader->buffer);
-			return result;
-	}
-
-	skip_bom(reader);
-	ldot = strrchr(key, '.');
-	name = ldot + 1;
-	section = git__strndup(key, ldot - key);
-
-	while (!reader->eof) {
-		c = reader_peek(reader, SKIP_WHITESPACE);
-
-		if (c == '\n') { /* We've arrived at the end of the file */
-			break;
-
-		} else if (c == '[') { /* section header, new section begins */
-			/*
-			 * We set both positions to the current one in case we
-			 * need to add a variable to the end of a section. In that
-			 * case, we want both variables to point just before the
-			 * new section. If we actually want to replace it, the
-			 * default case will take care of updating them.
-			 */
-			pre_end = post_start = reader->read_ptr;
-
-			git__free(current_section);
-			current_section = NULL;
-			if (parse_section_header(reader, &current_section) < 0)
-				goto rewrite_fail;
-
-			/* Keep track of when it stops matching */
-			last_section_matched = section_matches;
-			section_matches = !strcmp(current_section, section);
-		}
-
-		else if (c == ';' || c == '#') {
-			reader_consume_line(reader);
-		}
-
-		else {
-			/*
-			 * If the section doesn't match, but the last section did,
-			 * it means we need to add a variable (so skip the line
-			 * otherwise). If both the section and name match, we need
-			 * to overwrite the variable (so skip the line
-			 * otherwise). pre_end needs to be updated each time so we
-			 * don't loose that information, but we only need to
-			 * update post_start if we're going to use it in this
-			 * iteration.
-			 * If the section doesn't match and we are trying to delete an entry
-			 * (value == NULL), we must continue searching; there may be another
-			 * matching section later.
-			 */
-			if (!section_matches) {
-				if (!last_section_matched || value == NULL) {
-					reader_consume_line(reader);
-					continue;
-				}
-			} else {
-				int has_matched = 0;
-				char *var_name, *var_value;
-
-				pre_end = reader->read_ptr;
-				if (parse_variable(reader, &var_name, &var_value) < 0)
-					goto rewrite_fail;
-
-				/* First try to match the name of the variable */
-				if (strcasecmp(name, var_name) == 0)
-					has_matched = 1;
-
-				/* If the name matches, and we have a regex to match the
-				 * value, try to match it */
-				if (has_matched && preg != NULL)
-					has_matched = (regexec(preg, var_value, 0, NULL, 0) == 0);
-
-				git__free(var_name);
-				git__free(var_value);
-
-				/* if there is no match, keep going */
-				if (!has_matched)
-					continue;
-
-				post_start = reader->read_ptr;
-			}
-
-			/* We've found the variable we wanted to change, so
-			 * write anything up to it */
-			git_filebuf_write(&file, write_start, pre_end - write_start);
-			preg_replaced = 1;
-
-			/* Then replace the variable. If the value is NULL, it
-			 * means we want to delete it, so don't write anything. */
-			if (value != NULL) {
-				const char *q = quotes_for_value(value);
-				git_filebuf_printf(&file, "\t%s = %s%s%s\n", name, q, value, q);
-			}
-
-			/*
-			 * If we have a multivar, we should keep looking for entries,
-			 * but only if we're in the right section. Otherwise we'll end up
-			 * looping on the edge of a matching and a non-matching section.
-			 */
-			if (section_matches && preg != NULL) {
-				write_start = post_start;
-				continue;
-			}
-
-			write_trailer = 1;
-			break; /* break from the loop */
-		}
-	}
-
-	/*
-	 * Being here can mean that
-	 *
-	 * 1) our section is the last one in the file and we're
-	 * adding a variable
-	 *
-	 * 2) we didn't find a section for us so we need to create it
-	 * ourselves.
-	 *
-	 * 3) we're setting a multivar with a regex, which means we
-	 * continue to search for matching values
-	 *
-	 * In the last case, if we've already replaced a value, we
-	 * want to write the rest of the file. Otherwise we need to write
-	 * out the whole file and then the new variable.
-	 */
-	if (write_trailer) {
-		/* Write out rest of the file */
-		git_filebuf_write(&file, post_start, reader->buffer.size - (post_start - data_start));
-	} else {
-		if (preg_replaced) {
-			git_filebuf_printf(&file, "\n%s", write_start);
-		} else {
-			const char *q;
-
-			git_filebuf_write(&file, reader->buffer.ptr, reader->buffer.size);
-
-			if (reader->buffer.size > 0 && *(reader->buffer.ptr + reader->buffer.size - 1) != '\n')
-				git_filebuf_write(&file, "\n", 1);
-
-			/* And now if we just need to add a variable */
-			if (!section_matches && write_section(&file, section) < 0)
-				goto rewrite_fail;
-
-			/* Sanity check: if we are here, and value is NULL, that means that somebody
-			 * touched the config file after our initial read. We should probably assert()
-			 * this, but instead we'll handle it gracefully with an error. */
-			if (value == NULL) {
-				giterr_set(GITERR_CONFIG,
-					"race condition when writing a config file (a cvar has been removed)");
-				goto rewrite_fail;
-			}
-
-			/* If we are here, there is at least a section line */
-			q = quotes_for_value(value);
-			git_filebuf_printf(&file, "\t%s = %s%s%s\n", name, q, value, q);
-		}
-	}
-
-	git__free(section);
-	git__free(current_section);
-
-	/* refresh stats - if this errors, then commit will error too */
-	(void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
-
-	result = git_filebuf_commit(&file);
-	git_buf_free(&reader->buffer);
-
-	return result;
-
-rewrite_fail:
-	git__free(section);
-	git__free(current_section);
-
-	git_filebuf_cleanup(&file);
-	git_buf_free(&reader->buffer);
-	return -1;
-}
-
 static const char *escapes = "ntb\"\\";
 static const char *escaped = "\n\t\b\"\\";
 
@@ -1813,3 +1420,384 @@ on_error:
 	git__free(line);
 	return -1;
 }
+
+static int config_parse(
+	struct reader *reader,
+	int (*on_section)(struct reader **reader, const char *current_section, void *data),
+	int (*on_variable)(struct reader **reader, const char *current_section, char *var_name, char *var_value, void *data),
+	int (*on_eof)(struct reader **reader, void *data),
+	void *data)
+{
+	char *current_section = NULL, *var_name, *var_value;
+	char c;
+	int result = 0;
+
+	skip_bom(reader);
+
+	while (result == 0 && !reader->eof) {
+		c = reader_peek(reader, SKIP_WHITESPACE);
+
+		switch (c) {
+		case '\n': /* EOF when peeking, set EOF in the reader to exit the loop */
+			reader->eof = 1;
+			break;
+
+		case '[': /* section header, new section begins */
+			git__free(current_section);
+			current_section = NULL;
+
+			if ((result = parse_section_header(reader, &current_section)) == 0 && on_section)
+				result = on_section(&reader, current_section, data);
+			break;
+
+		case ';':
+		case '#':
+			/* TODO: handle comments */
+			reader_consume_line(reader);
+			break;
+
+		default: /* assume variable declaration */
+			if ((result = parse_variable(reader, &var_name, &var_value)) == 0 && on_variable)
+				result = on_variable(&reader, current_section, var_name, var_value, data);
+			break;
+		}
+	}
+
+	if (on_eof)
+		result = on_eof(&reader, data);
+
+	git__free(current_section);
+	return result;
+}
+
+struct parse_data {
+	git_strmap *values;
+	diskfile_backend *cfg_file;
+	uint32_t reader_idx;
+	git_config_level_t level;
+	int depth;
+};
+
+static int read_on_variable(struct reader **reader, const char *current_section, char *var_name, char *var_value, void *data)
+{
+	struct parse_data *parse_data = (struct parse_data *)data;
+	git_buf buf = GIT_BUF_INIT;
+	cvar_t *var;
+	int result = 0;
+
+	git__strtolower(var_name);
+	git_buf_printf(&buf, "%s.%s", current_section, var_name);
+	git__free(var_name);
+
+	if (git_buf_oom(&buf)) {
+		git__free(var_value);
+		return -1;
+	}
+
+	var = git__calloc(1, sizeof(cvar_t));
+	GITERR_CHECK_ALLOC(var);
+	var->entry = git__calloc(1, sizeof(git_config_entry));
+	GITERR_CHECK_ALLOC(var->entry);
+
+	var->entry->name = git_buf_detach(&buf);
+	var->entry->value = var_value;
+	var->entry->level = parse_data->level;
+	var->included = !!parse_data->depth;
+
+	if ((result = append_entry(parse_data->values, var)) < 0)
+		return result;
+
+	result = 0;
+
+	/* Add or append the new config option */
+	if (!git__strcmp(var->entry->name, "include.path")) {
+		struct reader *r;
+		git_buf path = GIT_BUF_INIT;
+		char *dir;
+		uint32_t index;
+
+		r = git_array_alloc(parse_data->cfg_file->readers);
+		/* The reader may have been reallocated */
+		*reader = git_array_get(parse_data->cfg_file->readers, parse_data->reader_idx);
+		memset(r, 0, sizeof(struct reader));
+
+		if ((result = git_path_dirname_r(&path, (*reader)->file_path)) < 0)
+			return result;
+
+		/* We need to know our index in the array, as the next config_parse call may realloc */
+		index = git_array_size(parse_data->cfg_file->readers) - 1;
+		dir = git_buf_detach(&path);
+		result = included_path(&path, dir, var->entry->value);
+		git__free(dir);
+
+		if (result < 0)
+			return result;
+
+		r->file_path = git_buf_detach(&path);
+		git_buf_init(&r->buffer, 0);
+
+		result = git_futils_readbuffer_updated(
+			&r->buffer, r->file_path, &r->file_mtime, &r->file_size, NULL);
+
+		if (result == 0) {
+			result = config_read(parse_data->values, parse_data->cfg_file, r, parse_data->level, parse_data->depth+1);
+			r = git_array_get(parse_data->cfg_file->readers, index);
+			*reader = git_array_get(parse_data->cfg_file->readers, parse_data->reader_idx);
+		} else if (result == GIT_ENOTFOUND) {
+			giterr_clear();
+			result = 0;
+		}
+
+		git_buf_free(&r->buffer);
+	}
+
+	return result;
+}
+
+static int config_read(git_strmap *values, diskfile_backend *cfg_file, struct reader *reader, git_config_level_t level, int depth)
+{
+	struct parse_data parse_data;
+
+	if (depth >= MAX_INCLUDE_DEPTH) {
+		giterr_set(GITERR_CONFIG, "Maximum config include depth reached");
+		return -1;
+	}
+
+	/* Initialize the reading position */
+	reader->read_ptr = reader->buffer.ptr;
+	reader->eof = 0;
+
+	/* If the file is empty, there's nothing for us to do */
+	if (*reader->read_ptr == '\0')
+		return 0;
+
+	parse_data.values = values;
+	parse_data.cfg_file = cfg_file;
+	parse_data.reader_idx = git_array_size(cfg_file->readers) - 1;
+	parse_data.level = level;
+	parse_data.depth = depth;
+
+	return config_parse(reader, NULL, read_on_variable, NULL, &parse_data);
+}
+
+static int write_section(git_filebuf *file, const char *key)
+{
+	int result;
+	const char *dot;
+	git_buf buf = GIT_BUF_INIT;
+
+	/* All of this just for [section "subsection"] */
+	dot = strchr(key, '.');
+	git_buf_putc(&buf, '[');
+	if (dot == NULL) {
+		git_buf_puts(&buf, key);
+	} else {
+		char *escaped;
+		git_buf_put(&buf, key, dot - key);
+		escaped = escape_value(dot + 1);
+		GITERR_CHECK_ALLOC(escaped);
+		git_buf_printf(&buf, " \"%s\"", escaped);
+		git__free(escaped);
+	}
+	git_buf_puts(&buf, "]\n");
+
+	if (git_buf_oom(&buf))
+		return -1;
+
+	result = git_filebuf_write(file, git_buf_cstr(&buf), buf.size);
+	git_buf_free(&buf);
+
+	return result;
+}
+
+static const char *quotes_for_value(const char *value)
+{
+	const char *ptr;
+
+	if (value[0] == ' ' || value[0] == '\0')
+		return "\"";
+
+	for (ptr = value; *ptr; ++ptr) {
+		if (*ptr == ';' || *ptr == '#')
+			return "\"";
+	}
+
+	if (ptr[-1] == ' ')
+		return "\"";
+
+	return "";
+}
+
+struct write_data {
+	git_filebuf *file;
+	unsigned int in_section : 1,
+		preg_replaced : 1;
+	const char *section;
+	const char *name;
+	const regex_t *preg;
+	const char *value;
+};
+
+static int write_value(struct write_data *write_data)
+{
+	const char *q;
+	int result;
+
+	q = quotes_for_value(write_data->value);
+	result = git_filebuf_printf(write_data->file,
+		"\t%s = %s%s%s\n", write_data->name, q, write_data->value, q);
+
+	/* If we are updating a single name/value, we're done.  Setting `value`
+	 * to `NULL` will prevent us from trying to write it again later (in
+	 * `write_on_section`) if we see the same section repeated.
+	 */
+	if (!write_data->preg)
+		write_data->value = NULL;
+
+	return result;
+}
+
+static int write_on_section(struct reader **reader, const char *current_section, void *data)
+{
+	struct write_data *write_data = (struct write_data *)data;
+	int result = 0;
+
+	/* If we were previously in the correct section (but aren't anymore)
+	 * and haven't written our value (for a simple name/value set, not
+	 * a multivar), then append it to the end of the section before writing
+	 * the new one.
+	 */
+	if (write_data->in_section && !write_data->preg && write_data->value)
+		result = write_value(write_data);
+
+	write_data->in_section = strcmp(current_section, write_data->section) == 0;
+
+	/* todo: no, write what's there */
+	if (!result)
+		result = write_section(write_data->file, current_section);
+
+	return result;
+}
+
+static int write_on_variable(struct reader **reader, const char *current_section, char *var_name, char *var_value, void *data)
+{
+	struct write_data *write_data = (struct write_data *)data;
+	bool has_matched = false;
+	int result = 0;
+
+	/* See if we are to update this name/value pair; first examine name */
+	if (write_data->in_section &&
+		strcasecmp(write_data->name, var_name) == 0)
+		has_matched = true;
+
+	/* If we have a regex to match the value, see if it matches */
+	if (has_matched && write_data->preg != NULL)
+		has_matched = (regexec(write_data->preg, var_value, 0, NULL, 0) == 0);
+
+	// TODO: do this
+//	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.
+	 */
+	if (!has_matched) {
+		// TODO: write write write
+		const char *q = quotes_for_value(var_value);
+		return git_filebuf_printf(write_data->file, "\t%s = %s%s%s\n", var_name, q, var_value, q);
+	}
+
+	write_data->preg_replaced = 1;
+
+	/* If value is NULL, we are deleting this value; write nothing. */
+	if (!write_data->value)
+		return 0;
+
+	return write_value(write_data);
+}
+
+static int write_on_eof(struct reader **reader, void *data)
+{
+	struct write_data *write_data = (struct write_data *)data;
+	int result = 0;
+
+	/* If we are at the EOF and have not written our value (again, for a
+	 * simple name/value set, not a multivar) then we have never seen the
+	 * section in question and should create a new section and write the
+	 * value.
+	 */
+	if ((!write_data->preg || !write_data->preg_replaced) && write_data->value) {
+		if ((result = write_section(write_data->file, write_data->section)) == 0)
+			result = write_value(write_data);
+	}
+
+	return result;
+}
+
+/*
+ * This is pretty much the parsing, except we write out anything we don't have
+ */
+static int config_write(diskfile_backend *cfg, const char *key, const regex_t *preg, const char* value)
+{
+	int result;
+	int section_matches = 0, last_section_matched = 0, preg_replaced = 0, write_trailer = 0;
+	const char *pre_end = NULL, *post_start = NULL, *data_start;
+	char *current_section = NULL, *section, *name, *ldot;
+	git_filebuf file = GIT_FILEBUF_INIT;
+	struct reader *reader = git_array_get(cfg->readers, 0);
+	struct write_data write_data;
+
+	/* TODO: take the lock before reading */
+
+	/* We need to read in our own config file */
+	result = git_futils_readbuffer(&reader->buffer, cfg->file_path);
+
+	/* Initialise the reading position */
+	if (result == GIT_ENOTFOUND) {
+		reader->read_ptr = NULL;
+		reader->eof = 1;
+		data_start = NULL;
+		git_buf_clear(&reader->buffer);
+	} else if (result == 0) {
+		reader->read_ptr = reader->buffer.ptr;
+		reader->eof = 0;
+		data_start = reader->read_ptr;
+	} else {
+		return -1; /* OS error when reading the file */
+	}
+
+	/* Lock the file */
+	if ((result = git_filebuf_open(
+		&file, cfg->file_path, 0, GIT_CONFIG_FILE_MODE)) < 0) {
+			git_buf_free(&reader->buffer);
+			return result;
+	}
+
+	ldot = strrchr(key, '.');
+	name = ldot + 1;
+	section = git__strndup(key, ldot - key);
+
+	write_data.file = &file;
+	write_data.section = section;
+	write_data.in_section = 0;
+	write_data.preg_replaced = 0;
+	write_data.name = name;
+	write_data.preg = preg;
+	write_data.value = value;
+
+	if ((result = config_parse(reader, write_on_section, write_on_variable, write_on_eof, &write_data)) < 0) {
+		git_filebuf_cleanup(&file);
+		goto done;
+	}
+
+	/* refresh stats - if this errors, then commit will error too */
+	(void)git_filebuf_stats(&reader->file_mtime, &reader->file_size, &file);
+
+	result = git_filebuf_commit(&file);
+	git_buf_free(&reader->buffer);
+
+done:
+	git_buf_free(&reader->buffer);
+	return result;
+}
+
diff --git a/tests/config/write.c b/tests/config/write.c
index 5e4e7e12b..60d900535 100644
--- a/tests/config/write.c
+++ b/tests/config/write.c
@@ -181,6 +181,43 @@ void test_config_write__overwrite_value_with_duplicate_header(void)
 	git_config_free(cfg);
 }
 
+void test_config_write__overwrite_multivar_within_duplicate_header(void)
+{
+	const char *file_name  = "config-duplicate-header";
+	const char *entry_name = "remote.origin.url";
+	git_config *cfg;
+	git_config_entry *entry;
+
+	/* This config can occur after removing and re-adding the origin remote */
+	const char *file_content =
+		"[remote \"origin\"]\n"		\
+		"	url = \"bar\"\n"		\
+		"[branch \"master\"]\n"		\
+		"	remote = \"origin\"\n"	\
+		"[remote \"origin\"]\n"		\
+		"	url = \"foo\"\n";
+
+	/* Write the test config and make sure the expected entry exists */
+	cl_git_mkfile(file_name, file_content);
+	cl_git_pass(git_config_open_ondisk(&cfg, file_name));
+	cl_git_pass(git_config_get_entry(&entry, cfg, entry_name));
+
+	/* Update that entry */
+	cl_git_pass(git_config_set_multivar(cfg, entry_name, "", "newurl"));
+
+	/* Reopen the file and make sure the entry was updated */
+	git_config_entry_free(entry);
+	git_config_free(cfg);
+	cl_git_pass(git_config_open_ondisk(&cfg, file_name));
+	cl_git_pass(git_config_get_entry(&entry, cfg, entry_name));
+
+	cl_assert_equal_s("newurl", entry->value);
+
+	/* Cleanup */
+	git_config_entry_free(entry);
+	git_config_free(cfg);
+}
+
 void test_config_write__write_subsection(void)
 {
 	git_config *cfg;