Merge branch 'ab/leakfix'

Plug various memory leaks.

* ab/leakfix:
  pull: fix a "struct oid_array" memory leak
  cat-file: fix a common "struct object_context" memory leak
  gc: fix a memory leak
  checkout: avoid "struct unpack_trees_options" leak
  merge-file: fix memory leaks on error path
  merge-file: refactor for subsequent memory leak fix
  cat-file: fix a memory leak in --batch-command mode
  revert: free "struct replay_opts" members
  submodule.c: free() memory from xgetcwd()
  clone: fix memory leak in wanted_peer_refs()
  check-ref-format: fix trivial memory leak

Author: gitster

builtin/cat-file.c

@@ -71,6 +71,7 @@ static int stream_blob(const struct object_id *oid)
 static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			int unknown_type)
 {
+	int ret;
 	struct object_id oid;
 	enum object_type type;
 	char *buf;
@@ -106,7 +107,8 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		if (sb.len) {
 			printf("%s\n", sb.buf);
 			strbuf_release(&sb);
-			return 0;
+			ret = 0;
+			goto cleanup;
 		}
 		break;
 
@@ -115,16 +117,19 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		if (oid_object_info_extended(the_repository, &oid, &oi, flags) < 0)
 			die("git cat-file: could not get object info");
 		printf("%"PRIuMAX"\n", (uintmax_t)size);
-		return 0;
+		ret = 0;
+		goto cleanup;
 
 	case 'e':
 		return !has_object_file(&oid);
 
 	case 'w':
 
 		if (filter_object(path, obj_context.mode,
-				  &oid, &buf, &size))
-			return -1;
+				  &oid, &buf, &size)) {
+			ret = -1;
+			goto cleanup;
+		}
 		break;
 
 	case 'c':
@@ -143,11 +148,14 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			const char *ls_args[3] = { NULL };
 			ls_args[0] =  "ls-tree";
 			ls_args[1] =  obj_name;
-			return cmd_ls_tree(2, ls_args, NULL);
+			ret = cmd_ls_tree(2, ls_args, NULL);
+			goto cleanup;
 		}
 
-		if (type == OBJ_BLOB)
-			return stream_blob(&oid);
+		if (type == OBJ_BLOB) {
+			ret = stream_blob(&oid);
+			goto cleanup;
+		}
 		buf = read_object_file(&oid, &type, &size);
 		if (!buf)
 			die("Cannot read object %s", obj_name);
@@ -172,8 +180,10 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 			} else
 				oidcpy(&blob_oid, &oid);
 
-			if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB)
-				return stream_blob(&blob_oid);
+			if (oid_object_info(the_repository, &blob_oid, NULL) == OBJ_BLOB) {
+				ret = stream_blob(&blob_oid);
+				goto cleanup;
+			}
 			/*
 			 * we attempted to dereference a tag to a blob
 			 * and failed; there may be new dereference
@@ -193,9 +203,11 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
 		die("git cat-file %s: bad file", obj_name);
 
 	write_or_die(1, buf, size);
+	ret = 0;
+cleanup:
 	free(buf);
 	free(obj_context.path);
-	return 0;
+	return ret;
 }
 
 struct expand_data {
@@ -655,6 +667,7 @@ static void batch_objects_command(struct batch_options *opt,
 		free_cmds(queued_cmd, &nr);
 	}
 
+	free_cmds(queued_cmd, &nr);
 	free(queued_cmd);
 	strbuf_release(&input);
 }

builtin/check-ref-format.c

@@ -57,6 +57,8 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 	int normalize = 0;
 	int flags = 0;
 	const char *refname;
+	char *to_free = NULL;
+	int ret = 1;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(builtin_check_ref_format_usage);
@@ -81,11 +83,14 @@ int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 
 	refname = argv[i];
 	if (normalize)
-		refname = collapse_slashes(refname);
+		refname = to_free = collapse_slashes(refname);
 	if (check_refname_format(refname, flags))
-		return 1;
+		goto cleanup;
 	if (normalize)
 		printf("%s\n", refname);
 
-	return 0;
+	ret = 0;
+cleanup:
+	free(to_free);
+	return ret;
 }

builtin/checkout.c

@@ -710,6 +710,26 @@ static void setup_branch_path(struct branch_info *branch)
 	branch->path = strbuf_detach(&buf, NULL);
 }
 
+static void init_topts(struct unpack_trees_options *topts, int merge,
+		       int show_progress, int overwrite_ignore,
+		       struct commit *old_commit)
+{
+	memset(topts, 0, sizeof(*topts));
+	topts->head_idx = -1;
+	topts->src_index = &the_index;
+	topts->dst_index = &the_index;
+
+	setup_unpack_trees_porcelain(topts, "checkout");
+
+	topts->initial_checkout = is_cache_unborn();
+	topts->update = 1;
+	topts->merge = 1;
+	topts->quiet = merge && old_commit;
+	topts->verbose_update = show_progress;
+	topts->fn = twoway_merge;
+	topts->preserve_ignored = !overwrite_ignore;
+}
+
 static int merge_working_tree(const struct checkout_opts *opts,
 			      struct branch_info *old_branch_info,
 			      struct branch_info *new_branch_info,
@@ -740,13 +760,6 @@ static int merge_working_tree(const struct checkout_opts *opts,
 		struct unpack_trees_options topts;
 		const struct object_id *old_commit_oid;
 
-		memset(&topts, 0, sizeof(topts));
-		topts.head_idx = -1;
-		topts.src_index = &the_index;
-		topts.dst_index = &the_index;
-
-		setup_unpack_trees_porcelain(&topts, "checkout");
-
 		refresh_cache(REFRESH_QUIET);
 
 		if (unmerged_cache()) {
@@ -755,17 +768,12 @@ static int merge_working_tree(const struct checkout_opts *opts,
 		}
 
 		/* 2-way merge to the new branch */
-		topts.initial_checkout = is_cache_unborn();
-		topts.update = 1;
-		topts.merge = 1;
-		topts.quiet = opts->merge && old_branch_info->commit;
-		topts.verbose_update = opts->show_progress;
-		topts.fn = twoway_merge;
+		init_topts(&topts, opts->merge, opts->show_progress,
+			   opts->overwrite_ignore, old_branch_info->commit);
 		init_checkout_metadata(&topts.meta, new_branch_info->refname,
 				       new_branch_info->commit ?
 				       &new_branch_info->commit->object.oid :
 				       &new_branch_info->oid, NULL);
-		topts.preserve_ignored = !opts->overwrite_ignore;
 
 		old_commit_oid = old_branch_info->commit ?
 			&old_branch_info->commit->object.oid :

builtin/clone.c

@@ -494,6 +494,7 @@ static struct ref *wanted_peer_refs(const struct ref *refs,
 			/* if --branch=tag, pull the requested tag explicitly */
 			get_fetch_map(remote_head, tag_refspec, &tail, 0);
 		}
+		free_refs(remote_head);
 	} else {
 		int i;
 		for (i = 0; i < refspec->nr; i++)

builtin/gc.c

@@ -168,9 +168,15 @@ struct maintenance_run_opts;
 static int maintenance_task_pack_refs(MAYBE_UNUSED struct maintenance_run_opts *opts)
 {
 	struct strvec pack_refs_cmd = STRVEC_INIT;
+	int ret;
+
 	strvec_pushl(&pack_refs_cmd, "pack-refs", "--all", "--prune", NULL);
 
-	return run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);
+	ret = run_command_v_opt(pack_refs_cmd.v, RUN_GIT_CMD);
+
+	strvec_clear(&pack_refs_cmd);
+
+	return ret;
 }
 
 static int too_many_loose_objects(void)

builtin/merge-file.c

@@ -25,10 +25,10 @@ static int label_cb(const struct option *opt, const char *arg, int unset)
 
 int cmd_merge_file(int argc, const char **argv, const char *prefix)
 {
-	const char *names[3] = { NULL, NULL, NULL };
-	mmfile_t mmfs[3];
-	mmbuffer_t result = {NULL, 0};
-	xmparam_t xmp = {{0}};
+	const char *names[3] = { 0 };
+	mmfile_t mmfs[3] = { 0 };
+	mmbuffer_t result = { 0 };
+	xmparam_t xmp = { 0 };
 	int ret = 0, i = 0, to_stdout = 0;
 	int quiet = 0;
 	struct option options[] = {
@@ -71,31 +71,31 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 
 	for (i = 0; i < 3; i++) {
 		char *fname;
-		int ret;
+		mmfile_t *mmf = mmfs + i;
 
 		if (!names[i])
 			names[i] = argv[i];
 
 		fname = prefix_filename(prefix, argv[i]);
-		ret = read_mmfile(mmfs + i, fname);
+
+		if (read_mmfile(mmf, fname))
+			ret = -1;
+		else if (mmf->size > MAX_XDIFF_SIZE ||
+			 buffer_is_binary(mmf->ptr, mmf->size))
+			ret = error("Cannot merge binary files: %s",
+				    argv[i]);
+
 		free(fname);
 		if (ret)
-			return -1;
+			goto cleanup;
 
-		if (mmfs[i].size > MAX_XDIFF_SIZE ||
-		    buffer_is_binary(mmfs[i].ptr, mmfs[i].size))
-			return error("Cannot merge binary files: %s",
-					argv[i]);
 	}
 
 	xmp.ancestor = names[1];
 	xmp.file1 = names[0];
 	xmp.file2 = names[2];
 	ret = xdl_merge(mmfs + 1, mmfs + 0, mmfs + 2, &xmp, &result);
 
-	for (i = 0; i < 3; i++)
-		free(mmfs[i].ptr);
-
 	if (ret >= 0) {
 		const char *filename = argv[0];
 		char *fpath = prefix_filename(prefix, argv[0]);
@@ -116,5 +116,9 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	if (ret > 127)
 		ret = 127;
 
+cleanup:
+	for (i = 0; i < 3; i++)
+		free(mmfs[i].ptr);
+
 	return ret;
 }

builtin/pull.c

@@ -990,6 +990,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	int rebase_unspecified = 0;
 	int can_ff;
 	int divergent;
+	int ret;
 
 	if (!getenv("GIT_REFLOG_ACTION"))
 		set_reflog_message(argc, argv);
@@ -1100,7 +1101,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (is_null_oid(&orig_head)) {
 		if (merge_heads.nr > 1)
 			die(_("Cannot merge multiple branches into empty head."));
-		return pull_into_void(merge_heads.oid, &curr_head);
+		ret = pull_into_void(merge_heads.oid, &curr_head);
+		goto cleanup;
 	}
 	if (merge_heads.nr > 1) {
 		if (opt_rebase)
@@ -1125,8 +1127,6 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	}
 
 	if (opt_rebase) {
-		int ret = 0;
-
 		struct object_id newbase;
 		struct object_id upstream;
 		get_rebase_newbase_and_upstream(&newbase, &upstream, &curr_head,
@@ -1149,12 +1149,16 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
 			ret = rebase_submodules();
 
-		return ret;
+		goto cleanup;
 	} else {
-		int ret = run_merge();
+		ret = run_merge();
 		if (!ret && (recurse_submodules == RECURSE_SUBMODULES_ON ||
 			     recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND))
 			ret = update_submodules();
-		return ret;
+		goto cleanup;
 	}
+
+cleanup:
+	oid_array_clear(&merge_heads);
+	return ret;
 }

builtin/revert.c

@@ -246,6 +246,9 @@ int cmd_revert(int argc, const char **argv, const char *prefix)
 	res = run_sequencer(argc, argv, &opts);
 	if (res < 0)
 		die(_("revert failed"));
+	if (opts.revs)
+		release_revisions(opts.revs);
+	free(opts.revs);
 	return res;
 }
 

submodule.c

@@ -2388,7 +2388,7 @@ int get_superproject_working_tree(struct strbuf *buf)
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf sb = STRBUF_INIT;
 	struct strbuf one_up = STRBUF_INIT;
-	const char *cwd = xgetcwd();
+	char *cwd = xgetcwd();
 	int ret = 0;
 	const char *subpath;
 	int code;
@@ -2451,6 +2451,7 @@ int get_superproject_working_tree(struct strbuf *buf)
 		ret = 1;
 		free(super_wt);
 	}
+	free(cwd);
 	strbuf_release(&sb);
 
 	code = finish_command(&cp);

t/t0028-working-tree-encoding.sh

@@ -5,6 +5,7 @@ test_description='working-tree-encoding conversion via gitattributes'
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 
+TEST_PASSES_SANITIZE_LEAK=true
 . ./test-lib.sh
 . "$TEST_DIRECTORY/lib-encoding.sh"