Merge branch 'ps/leakfixes-part-8'

More leakfixes.

* ps/leakfixes-part-8: (23 commits)
  builtin/send-pack: fix leaking list of push options
  remote: fix leaking push reports
  t/helper: fix leaks in proc-receive helper
  pack-write: fix return parameter of `write_rev_file_order()`
  revision: fix leaking saved parents
  revision: fix memory leaks when rewriting parents
  midx-write: fix leaking buffer
  pack-bitmap-write: fix leaking OID array
  pseudo-merge: fix leaking strmap keys
  pseudo-merge: fix various memory leaks
  line-log: fix several memory leaks
  diff: improve lifecycle management of diff queues
  builtin/revert: fix leaking `gpg_sign` and `strategy` config
  t/helper: fix leaking repository in partial-clone helper
  builtin/clone: fix leaking repo state when cloning with bundle URIs
  builtin/pack-redundant: fix various memory leaks
  builtin/stash: fix leaking `pathspec_from_file`
  submodule: fix leaking submodule entry list
  wt-status: fix leaking buffer with sparse directories
  shell: fix leaking strings
  ...

Author: gitster

bloom.c

@@ -476,8 +476,6 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
 				*last_slash = '\0';
 
 			} while (*path);
-
-			diff_free_filepair(diff_queued_diff.queue[i]);
 		}
 
 		if (hashmap_get_size(&pathmap) > settings->max_changed_paths) {
@@ -508,8 +506,6 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
 	cleanup:
 		hashmap_clear_and_free(&pathmap, struct pathmap_hash_entry, entry);
 	} else {
-		for (i = 0; i < diff_queued_diff.nr; i++)
-			diff_free_filepair(diff_queued_diff.queue[i]);
 		init_truncated_large_filter(filter, settings->hash_version);
 
 		if (computed)
@@ -519,9 +515,7 @@ struct bloom_filter *get_or_compute_bloom_filter(struct repository *r,
 	if (computed)
 		*computed |= BLOOM_COMPUTED;
 
-	free(diff_queued_diff.queue);
-	DIFF_QUEUE_CLEAR(&diff_queued_diff);
-
+	diff_queue_clear(&diff_queued_diff);
 	return filter;
 }
 

branch.c

@@ -738,6 +738,7 @@ static int submodule_create_branch(struct repository *r,
 
 	strbuf_release(&child_err);
 	strbuf_release(&out_buf);
+	free(out_prefix);
 	return ret;
 }
 
@@ -794,7 +795,7 @@ void create_branches_recursively(struct repository *r, const char *name,
 	create_branch(r, name, start_committish, force, 0, reflog, quiet,
 		      BRANCH_TRACK_NEVER, dry_run);
 	if (dry_run)
-		return;
+		goto out;
 	/*
 	 * NEEDSWORK If tracking was set up in the superproject but not the
 	 * submodule, users might expect "git branch --recurse-submodules" to
@@ -815,8 +816,11 @@ void create_branches_recursively(struct repository *r, const char *name,
 			die(_("submodule '%s': cannot create branch '%s'"),
 			    submodule_entry_list.entries[i].submodule->name,
 			    name);
-		repo_clear(submodule_entry_list.entries[i].repo);
 	}
+
+out:
+	submodule_entry_list_release(&submodule_entry_list);
+	free(branch_point);
 }
 
 void remove_merge_branch_state(struct repository *r)

builtin/annotate.c

@@ -15,13 +15,23 @@ int cmd_annotate(int argc,
 		 struct repository *repo UNUSED)
 {
 	struct strvec args = STRVEC_INIT;
-	int i;
+	const char **args_copy;
+	int ret;
 
 	strvec_pushl(&args, "annotate", "-c", NULL);
-
-	for (i = 1; i < argc; i++) {
+	for (int i = 1; i < argc; i++)
 		strvec_push(&args, argv[i]);
-	}
 
-	return cmd_blame(args.nr, args.v, prefix, the_repository);
+	/*
+	 * `cmd_blame()` ends up modifying the array, which causes memory leaks
+	 * if we didn't copy the array here.
+	 */
+	CALLOC_ARRAY(args_copy, args.nr + 1);
+	COPY_ARRAY(args_copy, args.v, args.nr);
+
+	ret = cmd_blame(args.nr, args_copy, prefix, the_repository);
+
+	strvec_clear(&args);
+	free(args_copy);
+	return ret;
 }

builtin/clone.c

@@ -1403,8 +1403,17 @@ int cmd_clone(int argc,
 	 * data from the --bundle-uri option.
 	 */
 	if (bundle_uri) {
+		struct remote_state *state;
 		int has_heuristic = 0;
 
+		/*
+		 * We need to save the remote state as our remote's lifetime is
+		 * tied to it.
+		 */
+		state = the_repository->remote_state;
+		the_repository->remote_state = NULL;
+		repo_clear(the_repository);
+
 		/* At this point, we need the_repository to match the cloned repo. */
 		if (repo_init(the_repository, git_dir, work_tree))
 			warning(_("failed to initialize the repo, skipping bundle URI"));
@@ -1413,6 +1422,10 @@ int cmd_clone(int argc,
 				bundle_uri);
 		else if (has_heuristic)
 			git_config_set_gently("fetch.bundleuri", bundle_uri);
+
+		remote_state_clear(the_repository->remote_state);
+		free(the_repository->remote_state);
+		the_repository->remote_state = state;
 	} else {
 		/*
 		* Populate transport->got_remote_bundle_uri and
@@ -1422,12 +1435,26 @@ int cmd_clone(int argc,
 
 		if (transport->bundles &&
 		    hashmap_get_size(&transport->bundles->bundles)) {
+			struct remote_state *state;
+
+			/*
+			 * We need to save the remote state as our remote's
+			 * lifetime is tied to it.
+			 */
+			state = the_repository->remote_state;
+			the_repository->remote_state = NULL;
+			repo_clear(the_repository);
+
 			/* At this point, we need the_repository to match the cloned repo. */
 			if (repo_init(the_repository, git_dir, work_tree))
 				warning(_("failed to initialize the repo, skipping bundle URI"));
 			else if (fetch_bundle_list(the_repository,
 						   transport->bundles))
 				warning(_("failed to fetch advertised bundles"));
+
+			remote_state_clear(the_repository->remote_state);
+			free(the_repository->remote_state);
+			the_repository->remote_state = state;
 		} else {
 			clear_bundle_list(transport->bundles);
 			FREE_AND_NULL(transport->bundles);

builtin/index-pack.c

@@ -1505,7 +1505,7 @@ static void rename_tmp_packfile(const char **final_name,
 				struct strbuf *name, unsigned char *hash,
 				const char *ext, int make_read_only_if_same)
 {
-	if (*final_name != curr_name) {
+	if (!*final_name || strcmp(*final_name, curr_name)) {
 		if (!*final_name)
 			*final_name = odb_pack_name(name, hash, ext);
 		if (finalize_object_file(curr_name, *final_name))
@@ -1726,7 +1726,7 @@ int cmd_index_pack(int argc,
 {
 	int i, fix_thin_pack = 0, verify = 0, stat_only = 0, rev_index;
 	const char *curr_index;
-	const char *curr_rev_index = NULL;
+	char *curr_rev_index = NULL;
 	const char *index_name = NULL, *pack_name = NULL, *rev_index_name = NULL;
 	const char *keep_msg = NULL;
 	const char *promisor_msg = NULL;
@@ -1968,8 +1968,7 @@ int cmd_index_pack(int argc,
 		free((void *) curr_pack);
 	if (!index_name)
 		free((void *) curr_index);
-	if (!rev_index_name)
-		free((void *) curr_rev_index);
+	free(curr_rev_index);
 
 	/*
 	 * Let the caller know this pack is not self contained

builtin/pack-redundant.c

@@ -69,6 +69,15 @@ static inline void llist_init(struct llist **list)
 	(*list)->size = 0;
 }
 
+static void llist_free(struct llist *list)
+{
+	for (struct llist_item *i = list->front, *next; i; i = next) {
+		next = i->next;
+		llist_item_put(i);
+	}
+	free(list);
+}
+
 static struct llist * llist_copy(struct llist *list)
 {
 	struct llist *ret;
@@ -206,6 +215,14 @@ static inline struct pack_list * pack_list_insert(struct pack_list **pl,
 	return p;
 }
 
+static void pack_list_free(struct pack_list *pl)
+{
+	for (struct pack_list *next; pl; pl = next) {
+		next = pl->next;
+		free(pl);
+	}
+}
+
 static inline size_t pack_list_size(struct pack_list *pl)
 {
 	size_t ret = 0;
@@ -419,7 +436,8 @@ static void minimize(struct pack_list **min)
 
 	/* return if there are no objects missing from the unique set */
 	if (missing->size == 0) {
-		free(missing);
+		llist_free(missing);
+		pack_list_free(non_unique);
 		return;
 	}
 
@@ -434,6 +452,8 @@ static void minimize(struct pack_list **min)
 	}
 
 	while (non_unique) {
+		struct pack_list *next;
+
 		/* sort the non_unique packs, greater size of remaining_objects first */
 		sort_pack_list(&non_unique);
 		if (non_unique->remaining_objects->size == 0)
@@ -444,8 +464,14 @@ static void minimize(struct pack_list **min)
 		for (pl = non_unique->next; pl && pl->remaining_objects->size > 0;  pl = pl->next)
 			llist_sorted_difference_inplace(pl->remaining_objects, non_unique->remaining_objects);
 
-		non_unique = non_unique->next;
+		next = non_unique->next;
+		free(non_unique);
+		non_unique = next;
 	}
+
+	pack_list_free(non_unique);
+	llist_free(unique_pack_objects);
+	llist_free(missing);
 }
 
 static void load_all_objects(void)
@@ -565,7 +591,6 @@ static void load_all(void)
 int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, struct repository *repo UNUSED) {
 	int i; int i_still_use_this = 0; struct pack_list *min = NULL, *red, *pl;
 	struct llist *ignore;
-	struct object_id *oid;
 	char buf[GIT_MAX_HEXSZ + 2]; /* hex hash + \n + \0 */
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
@@ -625,11 +650,11 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
 	/* ignore objects given on stdin */
 	llist_init(&ignore);
 	if (!isatty(0)) {
+		struct object_id oid;
 		while (fgets(buf, sizeof(buf), stdin)) {
-			oid = xmalloc(sizeof(*oid));
-			if (get_oid_hex(buf, oid))
+			if (get_oid_hex(buf, &oid))
 				die("Bad object ID on stdin: %s", buf);
-			llist_insert_sorted_unique(ignore, oid, NULL);
+			llist_insert_sorted_unique(ignore, &oid, NULL);
 		}
 	}
 	llist_sorted_difference_inplace(all_objects, ignore);
@@ -671,5 +696,8 @@ int cmd_pack_redundant(int argc, const char **argv, const char *prefix UNUSED, s
 		fprintf(stderr, "%luMB of redundant packs in total.\n",
 			(unsigned long)pack_set_bytecount(red)/(1024*1024));
 
+	pack_list_free(red);
+	pack_list_free(min);
+	llist_free(ignore);
 	return 0;
 }

builtin/receive-pack.c

@@ -374,6 +374,7 @@ static void write_head_info(void)
 struct command {
 	struct command *next;
 	const char *error_string;
+	char *error_string_owned;
 	struct ref_push_report *report;
 	unsigned int skip_update:1,
 		     did_not_exist:1,
@@ -1083,7 +1084,7 @@ static int read_proc_receive_report(struct packet_reader *reader,
 		hint->run_proc_receive |= RUN_PROC_RECEIVE_RETURNED;
 		if (!strcmp(head, "ng")) {
 			if (p)
-				hint->error_string = xstrdup(p);
+				hint->error_string = hint->error_string_owned = xstrdup(p);
 			else
 				hint->error_string = "failed";
 			code = -1;
@@ -2054,6 +2055,8 @@ static void free_commands(struct command *commands)
 	while (commands) {
 		struct command *next = commands->next;
 
+		ref_push_report_free(commands->report);
+		free(commands->error_string_owned);
 		free(commands);
 		commands = next;
 	}

builtin/revert.c

@@ -110,6 +110,9 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 	const char * const * usage_str = revert_or_cherry_pick_usage(opts);
 	const char *me = action_name(opts);
 	const char *cleanup_arg = NULL;
+	const char sentinel_value;
+	const char *strategy = &sentinel_value;
+	const char *gpg_sign = &sentinel_value;
 	enum empty_action empty_opt = EMPTY_COMMIT_UNSPECIFIED;
 	int cmd = 0;
 	struct option base_options[] = {
@@ -125,10 +128,10 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 		OPT_CALLBACK('m', "mainline", opts, N_("parent-number"),
 			     N_("select mainline parent"), option_parse_m),
 		OPT_RERERE_AUTOUPDATE(&opts->allow_rerere_auto),
-		OPT_STRING(0, "strategy", &opts->strategy, N_("strategy"), N_("merge strategy")),
+		OPT_STRING(0, "strategy", &strategy, N_("strategy"), N_("merge strategy")),
 		OPT_STRVEC('X', "strategy-option", &opts->xopts, N_("option"),
 			N_("option for merge strategy")),
-		{ OPTION_STRING, 'S', "gpg-sign", &opts->gpg_sign, N_("key-id"),
+		{ OPTION_STRING, 'S', "gpg-sign", &gpg_sign, N_("key-id"),
 		  N_("GPG sign commit"), PARSE_OPT_OPTARG, NULL, (intptr_t) "" },
 		OPT_END()
 	};
@@ -240,8 +243,14 @@ static int run_sequencer(int argc, const char **argv, const char *prefix,
 		usage_with_options(usage_str, options);
 
 	/* These option values will be free()d */
-	opts->gpg_sign = xstrdup_or_null(opts->gpg_sign);
-	opts->strategy = xstrdup_or_null(opts->strategy);
+	if (gpg_sign != &sentinel_value) {
+		free(opts->gpg_sign);
+		opts->gpg_sign = xstrdup_or_null(gpg_sign);
+	}
+	if (strategy != &sentinel_value) {
+		free(opts->strategy);
+		opts->strategy = xstrdup_or_null(strategy);
+	}
 	if (!opts->strategy && getenv("GIT_TEST_MERGE_ALGORITHM"))
 		opts->strategy = xstrdup(getenv("GIT_TEST_MERGE_ALGORITHM"));
 	free(options);

builtin/send-pack.c

@@ -340,6 +340,7 @@ int cmd_send_pack(int argc,
 		/* stable plumbing output; do not modify or localize */
 		fprintf(stderr, "Everything up-to-date\n");
 
+	string_list_clear(&push_options, 0);
 	free_refs(remote_refs);
 	free_refs(local_refs);
 	refspec_clear(&rs);

builtin/stash.c

@@ -1759,7 +1759,7 @@ static int push_stash(int argc, const char **argv, const char *prefix,
 	int quiet = 0;
 	int pathspec_file_nul = 0;
 	const char *stash_msg = NULL;
-	const char *pathspec_from_file = NULL;
+	char *pathspec_from_file = NULL;
 	struct pathspec ps;
 	struct option options[] = {
 		OPT_BOOL('k', "keep-index", &keep_index,
@@ -1821,7 +1821,9 @@ static int push_stash(int argc, const char **argv, const char *prefix,
 
 	ret = do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode,
 			    include_untracked, only_staged);
+
 	clear_pathspec(&ps);
+	free(pathspec_from_file);
 	return ret;
 }