Merge branch 'jk/fetch-prefetch-double-free-fix'

Double-free fix.

* jk/fetch-prefetch-double-free-fix:
  refspec: store raw refspecs inside refspec_item
  refspec: drop separate raw_nr count
  fetch: adjust refspec->raw_nr when filtering prefetch refspecs

builtin/fetch.c

@@ -454,14 +454,10 @@ static void filter_prefetch_refspec(struct refspec *rs)
 				 ref_namespace[NAMESPACE_TAGS].ref))) {
 			int j;
 
-			free(rs->items[i].src);
-			free(rs->items[i].dst);
-			free(rs->raw[i]);
+			refspec_item_clear(&rs->items[i]);
 
-			for (j = i + 1; j < rs->nr; j++) {
+			for (j = i + 1; j < rs->nr; j++)
 				rs->items[j - 1] = rs->items[j];
-				rs->raw[j - 1] = rs->raw[j];
-			}
 			rs->nr--;
 			i--;
 			continue;

builtin/remote.c

@@ -377,7 +377,7 @@ static int get_ref_states(const struct ref *remote_refs, struct ref_states *stat
 	for (i = 0; i < states->remote->fetch.nr; i++)
 		if (get_fetch_map(remote_refs, &states->remote->fetch.items[i], &tail, 1))
 			die(_("Could not get fetch map for refspec %s"),
-				states->remote->fetch.raw[i]);
+				states->remote->fetch.items[i].raw);
 
 	for (ref = fetch_map; ref; ref = ref->next) {
 		if (omit_name_by_refspec(ref->name, &states->remote->fetch))
@@ -633,12 +633,12 @@ static int migrate_file(struct remote *remote)
 		git_config_set_multivar(buf.buf, remote->url.v[i], "^$", 0);
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s.push", remote->name);
-	for (i = 0; i < remote->push.raw_nr; i++)
-		git_config_set_multivar(buf.buf, remote->push.raw[i], "^$", 0);
+	for (i = 0; i < remote->push.nr; i++)
+		git_config_set_multivar(buf.buf, remote->push.items[i].raw, "^$", 0);
 	strbuf_reset(&buf);
 	strbuf_addf(&buf, "remote.%s.fetch", remote->name);
-	for (i = 0; i < remote->fetch.raw_nr; i++)
-		git_config_set_multivar(buf.buf, remote->fetch.raw[i], "^$", 0);
+	for (i = 0; i < remote->fetch.nr; i++)
+		git_config_set_multivar(buf.buf, remote->fetch.items[i].raw, "^$", 0);
 	if (remote->origin == REMOTE_REMOTES)
 		unlink_or_warn(git_path("remotes/%s", remote->name));
 	else if (remote->origin == REMOTE_BRANCHES)
@@ -759,16 +759,16 @@ static int mv(int argc, const char **argv, const char *prefix)
 		goto out;
 	}
 
-	if (oldremote->fetch.raw_nr) {
+	if (oldremote->fetch.nr) {
 		strbuf_reset(&buf);
 		strbuf_addf(&buf, "remote.%s.fetch", rename.new_name);
 		git_config_set_multivar(buf.buf, NULL, NULL, CONFIG_FLAGS_MULTI_REPLACE);
 		strbuf_addf(&old_remote_context, ":refs/remotes/%s/", rename.old_name);
-		for (i = 0; i < oldremote->fetch.raw_nr; i++) {
+		for (i = 0; i < oldremote->fetch.nr; i++) {
 			char *ptr;
 
 			strbuf_reset(&buf2);
-			strbuf_addstr(&buf2, oldremote->fetch.raw[i]);
+			strbuf_addstr(&buf2, oldremote->fetch.items[i].raw);
 			ptr = strstr(buf2.buf, old_remote_context.buf);
 			if (ptr) {
 				refspec_updated = 1;

refspec.c

@@ -153,6 +153,7 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
 int refspec_item_init(struct refspec_item *item, const char *refspec, int fetch)
 {
 	memset(item, 0, sizeof(*item));
+	item->raw = xstrdup(refspec);
 	return parse_refspec(item, refspec, fetch);
 }
 
@@ -167,6 +168,7 @@ void refspec_item_clear(struct refspec_item *item)
 {
 	FREE_AND_NULL(item->src);
 	FREE_AND_NULL(item->dst);
+	FREE_AND_NULL(item->raw);
 	item->force = 0;
 	item->pattern = 0;
 	item->matching = 0;
@@ -179,31 +181,29 @@ void refspec_init(struct refspec *rs, int fetch)
 	rs->fetch = fetch;
 }
 
-static void refspec_append_nodup(struct refspec *rs, char *refspec)
+void refspec_append(struct refspec *rs, const char *refspec)
 {
 	struct refspec_item item;
 
 	refspec_item_init_or_die(&item, refspec, rs->fetch);
 
 	ALLOC_GROW(rs->items, rs->nr + 1, rs->alloc);
-	rs->items[rs->nr++] = item;
+	rs->items[rs->nr] = item;
 
-	ALLOC_GROW(rs->raw, rs->raw_nr + 1, rs->raw_alloc);
-	rs->raw[rs->raw_nr++] = refspec;
-}
-
-void refspec_append(struct refspec *rs, const char *refspec)
-{
-	refspec_append_nodup(rs, xstrdup(refspec));
+	rs->nr++;
 }
 
 void refspec_appendf(struct refspec *rs, const char *fmt, ...)
 {
 	va_list ap;
+	char *buf;
 
 	va_start(ap, fmt);
-	refspec_append_nodup(rs, xstrvfmt(fmt, ap));
+	buf = xstrvfmt(fmt, ap);
 	va_end(ap);
+
+	refspec_append(rs, buf);
+	free(buf);
 }
 
 void refspec_appendn(struct refspec *rs, const char **refspecs, int nr)
@@ -224,12 +224,6 @@ void refspec_clear(struct refspec *rs)
 	rs->alloc = 0;
 	rs->nr = 0;
 
-	for (i = 0; i < rs->raw_nr; i++)
-		free(rs->raw[i]);
-	FREE_AND_NULL(rs->raw);
-	rs->raw_alloc = 0;
-	rs->raw_nr = 0;
-
 	rs->fetch = 0;
 }
 

refspec.h

@@ -26,6 +26,8 @@ struct refspec_item {
 
 	char *src;
 	char *dst;
+
+	char *raw;
 };
 
 #define REFSPEC_FETCH 1
@@ -43,10 +45,6 @@ struct refspec {
 	int alloc;
 	int nr;
 
-	char **raw;
-	int raw_alloc;
-	int raw_nr;
-
 	int fetch;
 };
 

submodule.c

@@ -1174,8 +1174,8 @@ static int push_submodule(const char *path,
 		if (remote->origin != REMOTE_UNCONFIGURED) {
 			int i;
 			strvec_push(&cp.args, remote->name);
-			for (i = 0; i < rs->raw_nr; i++)
-				strvec_push(&cp.args, rs->raw[i]);
+			for (i = 0; i < rs->nr; i++)
+				strvec_push(&cp.args, rs->items[i].raw);
 		}
 
 		prepare_submodule_repo_env(&cp.env);
@@ -1209,8 +1209,8 @@ static void submodule_push_check(const char *path, const char *head,
 	strvec_push(&cp.args, head);
 	strvec_push(&cp.args, remote->name);
 
-	for (i = 0; i < rs->raw_nr; i++)
-		strvec_push(&cp.args, rs->raw[i]);
+	for (i = 0; i < rs->nr; i++)
+		strvec_push(&cp.args, rs->items[i].raw);
 
 	prepare_submodule_repo_env(&cp.env);
 	cp.git_cmd = 1;

t/t5582-fetch-negative-refspec.sh

@@ -283,4 +283,8 @@ test_expect_success '--prefetch succeeds when refspec becomes empty' '
 	git -C one fetch --prefetch
 '
 
+test_expect_success '--prefetch succeeds with empty command line refspec' '
+	git -C one fetch --prefetch origin +refs/tags/extra
+'
+
 test_done