Merge branch 'ps/leakfixes-part-5'

Even more leak fixes.

* ps/leakfixes-part-5:
  transport: fix leaking negotiation tips
  transport: fix leaking arguments when fetching from bundle
  builtin/fetch: fix leaking transaction with `--atomic`
  remote: fix leaking peer ref when expanding refmap
  remote: fix leaks when matching refspecs
  remote: fix leaking config strings
  builtin/fetch-pack: fix leaking refs
  sideband: fix leaks when configuring sideband colors
  builtin/send-pack: fix leaking refspecs
  transport: fix leaking OID arrays in git:// transport data
  t/helper: fix leaking multi-pack-indices in "read-midx"
  builtin/repack: fix leaks when computing packs to repack
  midx-write: fix leaking hashfile on error cases
  builtin/archive: fix leaking `OPT_FILENAME()` value
  builtin/upload-archive: fix leaking args passed to `write_archive()`
  builtin/merge-tree: fix leaking `-X` strategy options
  pretty: fix leaking key/value separator buffer
  pretty: fix memory leaks when parsing pretty formats
  convert: fix leaks when resetting attributes
  mailinfo: fix leaking header data

Author: gitster

archive.c

@@ -736,6 +736,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	struct pretty_print_describe_status describe_status = {0};
 	struct pretty_print_context ctx = {0};
 	struct archiver_args args;
+	const char **argv_copy;
 	int rc;
 
 	git_config_get_bool("uploadarchive.allowunreachable", &remote_allow_unreachable);
@@ -749,6 +750,14 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	args.repo = repo;
 	args.prefix = prefix;
 	string_list_init_dup(&args.extra_files);
+
+	/*
+	 * `parse_archive_args()` modifies contents of `argv`, which is what we
+	 * want. Our callers may not want it though, so we create a copy here.
+	 */
+	DUP_ARRAY(argv_copy, argv, argc);
+	argv = argv_copy;
+
 	argc = parse_archive_args(argc, argv, &ar, &args, name_hint, remote);
 	if (!startup_info->have_repository) {
 		/*
@@ -767,6 +776,7 @@ int write_archive(int argc, const char **argv, const char *prefix,
 	string_list_clear_func(&args.extra_files, extra_file_info_clear);
 	free(args.refname);
 	clear_pathspec(&args.pathspec);
+	free(argv_copy);
 
 	return rc;
 }

builtin/archive.c

@@ -100,13 +100,16 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
 	if (output)
 		create_output_file(output);
 
-	if (remote)
-		return run_remote_archiver(argc, argv, remote, exec, output);
+	if (remote) {
+		ret = run_remote_archiver(argc, argv, remote, exec, output);
+		goto out;
+	}
 
 	setvbuf(stderr, NULL, _IOLBF, BUFSIZ);
 
 	ret = write_archive(argc, argv, prefix, the_repository, output, 0);
 
+out:
 	free(output);
 	return ret;
 }

builtin/bundle.c

@@ -221,7 +221,9 @@ static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)
 			 &extra_index_pack_args, 0) ||
 		list_bundle_refs(&header, argc, argv);
 	bundle_header_release(&header);
+
 cleanup:
+	strvec_clear(&extra_index_pack_args);
 	free(bundle_file);
 	return ret;
 }

builtin/fetch-pack.c

@@ -46,7 +46,7 @@ static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)
 {
 	int i, ret;
-	struct ref *ref = NULL;
+	struct ref *fetched_refs = NULL, *remote_refs = NULL;
 	const char *dest = NULL;
 	struct ref **sought = NULL;
 	int nr_sought = 0, alloc_sought = 0;
@@ -228,19 +228,20 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)
 	version = discover_version(&reader);
 	switch (version) {
 	case protocol_v2:
-		get_remote_refs(fd[1], &reader, &ref, 0, NULL, NULL,
+		get_remote_refs(fd[1], &reader, &remote_refs, 0, NULL, NULL,
 				args.stateless_rpc);
 		break;
 	case protocol_v1:
 	case protocol_v0:
-		get_remote_heads(&reader, &ref, 0, NULL, &shallow);
+		get_remote_heads(&reader, &remote_refs, 0, NULL, &shallow);
 		break;
 	case protocol_unknown_version:
 		BUG("unknown protocol version");
 	}
 
-	ref = fetch_pack(&args, fd, ref, sought, nr_sought,
+	fetched_refs = fetch_pack(&args, fd, remote_refs, sought, nr_sought,
 			 &shallow, pack_lockfiles_ptr, version);
+
 	if (pack_lockfiles.nr) {
 		int i;
 
@@ -260,7 +261,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)
 	if (finish_connect(conn))
 		return 1;
 
-	ret = !ref;
+	ret = !fetched_refs;
 
 	/*
 	 * If the heads to pull were given, we should have consumed
@@ -270,11 +271,14 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix UNUSED)
 	 */
 	ret |= report_unmatched_refs(sought, nr_sought);
 
-	while (ref) {
+	for (struct ref *ref = fetched_refs; ref; ref = ref->next)
 		printf("%s %s\n",
 		       oid_to_hex(&ref->old_oid), ref->name);
-		ref = ref->next;
-	}
 
+	for (size_t i = 0; i < nr_sought; i++)
+		free_one_ref(sought[i]);
+	free(sought);
+	free_refs(fetched_refs);
+	free_refs(remote_refs);
 	return ret;
 }

builtin/fetch.c

@@ -1731,11 +1731,8 @@ static int do_fetch(struct transport *transport,
 			goto cleanup;
 
 		retcode = ref_transaction_commit(transaction, &err);
-		if (retcode) {
-			ref_transaction_free(transaction);
-			transaction = NULL;
+		if (retcode)
 			goto cleanup;
-		}
 	}
 
 	commit_fetch_head(&fetch_head);
@@ -1803,8 +1800,11 @@ static int do_fetch(struct transport *transport,
 		if (transaction && ref_transaction_abort(transaction, &err) &&
 		    err.len)
 			error("%s", err.buf);
+		transaction = NULL;
 	}
 
+	if (transaction)
+		ref_transaction_free(transaction);
 	display_state_release(&display_state);
 	close_fetch_head(&fetch_head);
 	strbuf_release(&err);

builtin/merge-tree.c

@@ -533,6 +533,7 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 	int expected_remaining_argc;
 	int original_argc;
 	const char *merge_base = NULL;
+	int ret;
 
 	const char * const merge_tree_usage[] = {
 		N_("git merge-tree [--write-tree] [<options>] <branch1> <branch2>"),
@@ -625,7 +626,9 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 			strbuf_list_free(split);
 		}
 		strbuf_release(&buf);
-		return 0;
+
+		ret = 0;
+		goto out;
 	}
 
 	/* Figure out which mode to use */
@@ -664,7 +667,11 @@ int cmd_merge_tree(int argc, const char **argv, const char *prefix)
 
 	/* Do the relevant type of merge */
 	if (o.mode == MODE_REAL)
-		return real_merge(&o, merge_base, argv[0], argv[1], prefix);
+		ret = real_merge(&o, merge_base, argv[0], argv[1], prefix);
 	else
-		return trivial_merge(argv[0], argv[1], argv[2]);
+		ret = trivial_merge(argv[0], argv[1], argv[2]);
+
+out:
+	strvec_clear(&xopts);
+	return ret;
 }

builtin/repack.c

@@ -732,14 +732,23 @@ static void midx_included_packs(struct string_list *include,
 				struct pack_geometry *geometry)
 {
 	struct string_list_item *item;
+	struct strbuf buf = STRBUF_INIT;
+
+	for_each_string_list_item(item, &existing->kept_packs) {
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "%s.idx", item->string);
+		string_list_insert(include, buf.buf);
+	}
+
+	for_each_string_list_item(item, names) {
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "pack-%s.idx", item->string);
+		string_list_insert(include, buf.buf);
+	}
 
-	for_each_string_list_item(item, &existing->kept_packs)
-		string_list_insert(include, xstrfmt("%s.idx", item->string));
-	for_each_string_list_item(item, names)
-		string_list_insert(include, xstrfmt("pack-%s.idx", item->string));
 	if (geometry->split_factor) {
-		struct strbuf buf = STRBUF_INIT;
 		uint32_t i;
+
 		for (i = geometry->split; i < geometry->pack_nr; i++) {
 			struct packed_git *p = geometry->pack[i];
 
@@ -754,17 +763,21 @@ static void midx_included_packs(struct string_list *include,
 			if (!p->pack_local)
 				continue;
 
+			strbuf_reset(&buf);
 			strbuf_addstr(&buf, pack_basename(p));
 			strbuf_strip_suffix(&buf, ".pack");
 			strbuf_addstr(&buf, ".idx");
 
-			string_list_insert(include, strbuf_detach(&buf, NULL));
+			string_list_insert(include, buf.buf);
 		}
 	} else {
 		for_each_string_list_item(item, &existing->non_kept_packs) {
 			if (pack_is_marked_for_deletion(item))
 				continue;
-			string_list_insert(include, xstrfmt("%s.idx", item->string));
+
+			strbuf_reset(&buf);
+			strbuf_addf(&buf, "%s.idx", item->string);
+			string_list_insert(include, buf.buf);
 		}
 	}
 
@@ -784,8 +797,13 @@ static void midx_included_packs(struct string_list *include,
 		 */
 		if (pack_is_marked_for_deletion(item))
 			continue;
-		string_list_insert(include, xstrfmt("%s.idx", item->string));
+
+		strbuf_reset(&buf);
+		strbuf_addf(&buf, "%s.idx", item->string);
+		string_list_insert(include, buf.buf);
 	}
+
+	strbuf_release(&buf);
 }
 
 static int write_midx_included_packs(struct string_list *include,
@@ -1476,7 +1494,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
 		mark_packs_for_deletion(&existing, &names);
 
 	if (write_midx) {
-		struct string_list include = STRING_LIST_INIT_NODUP;
+		struct string_list include = STRING_LIST_INIT_DUP;
 		midx_included_packs(&include, &existing, &names, &geometry);
 
 		ret = write_midx_included_packs(&include, &geometry, &names,

builtin/send-pack.c

@@ -338,5 +338,6 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 
 	free_refs(remote_refs);
 	free_refs(local_refs);
+	refspec_clear(&rs);
 	return ret;
 }

builtin/upload-archive.c

@@ -22,6 +22,7 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
 {
 	struct strvec sent_argv = STRVEC_INIT;
 	const char *arg_cmd = "argument ";
+	int ret;
 
 	if (argc != 2 || !strcmp(argv[1], "-h"))
 		usage(upload_archive_usage);
@@ -46,8 +47,11 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix)
 	}
 
 	/* parse all options sent by the client */
-	return write_archive(sent_argv.nr, sent_argv.v, prefix,
-			     the_repository, NULL, 1);
+	ret = write_archive(sent_argv.nr, sent_argv.v, prefix,
+			    the_repository, NULL, 1);
+
+	strvec_clear(&sent_argv);
+	return ret;
 }
 
 __attribute__((format (printf, 1, 2)))

bundle.c

@@ -644,10 +644,8 @@ int unbundle(struct repository *r, struct bundle_header *header,
 	if (flags & VERIFY_BUNDLE_FSCK)
 		strvec_push(&ip.args, "--fsck-objects");
 
-	if (extra_index_pack_args) {
+	if (extra_index_pack_args)
 		strvec_pushv(&ip.args, extra_index_pack_args->v);
-		strvec_clear(extra_index_pack_args);
-	}
 
 	ip.in = bundle_fd;
 	ip.no_stdout = 1;

convert.c

@@ -1371,6 +1371,9 @@ void reset_parsed_attributes(void)
 	for (drv = user_convert; drv; drv = next) {
 		next = drv->next;
 		free((void *)drv->name);
+		free((void *)drv->smudge);
+		free((void *)drv->clean);
+		free((void *)drv->process);
 		free(drv);
 	}
 	user_convert = NULL;

mailinfo.c

@@ -1292,8 +1292,21 @@ void clear_mailinfo(struct mailinfo *mi)
 	strbuf_release(&mi->inbody_header_accum);
 	free(mi->message_id);
 
-	strbuf_list_free(mi->p_hdr_data);
-	strbuf_list_free(mi->s_hdr_data);
+	for (size_t i = 0; header[i]; i++) {
+		if (!mi->p_hdr_data[i])
+			continue;
+		strbuf_release(mi->p_hdr_data[i]);
+		free(mi->p_hdr_data[i]);
+	}
+	free(mi->p_hdr_data);
+
+	for (size_t i = 0; header[i]; i++) {
+		if (!mi->s_hdr_data[i])
+			continue;
+		strbuf_release(mi->s_hdr_data[i]);
+		free(mi->s_hdr_data[i]);
+	}
+	free(mi->s_hdr_data);
 
 	while (mi->content < mi->content_top) {
 		free(*(mi->content_top));

midx-write.c

@@ -1306,6 +1306,18 @@ static int write_midx_internal(const char *object_dir,
 		pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -
 					(pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);
 
+	if (ctx.nr - dropped_packs == 0) {
+		error(_("no pack files to index."));
+		result = 1;
+		goto cleanup;
+	}
+
+	if (!ctx.entries_nr) {
+		if (flags & MIDX_WRITE_BITMAP)
+			warning(_("refusing to write multi-pack .bitmap without any objects"));
+		flags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);
+	}
+
 	if (ctx.incremental) {
 		struct strbuf lock_name = STRBUF_INIT;
 
@@ -1331,18 +1343,6 @@ static int write_midx_internal(const char *object_dir,
 		f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));
 	}
 
-	if (ctx.nr - dropped_packs == 0) {
-		error(_("no pack files to index."));
-		result = 1;
-		goto cleanup;
-	}
-
-	if (!ctx.entries_nr) {
-		if (flags & MIDX_WRITE_BITMAP)
-			warning(_("refusing to write multi-pack .bitmap without any objects"));
-		flags &= ~(MIDX_WRITE_REV_INDEX | MIDX_WRITE_BITMAP);
-	}
-
 	cf = init_chunkfile(f);
 
 	add_chunk(cf, MIDX_CHUNKID_PACKNAMES, pack_name_concat_len,