Merge branch 'ps/leakfixes-part-3'

More leakfixes.

* ps/leakfixes-part-3: (24 commits)
  commit-reach: fix trivial memory leak when computing reachability
  convert: fix leaking config strings
  entry: fix leaking pathnames during delayed checkout
  object-name: fix leaking commit list items
  t/test-repository: fix leaking repository
  builtin/credential-cache: fix trivial leaks
  builtin/worktree: fix leaking derived branch names
  builtin/shortlog: fix various trivial memory leaks
  builtin/rerere: fix various trivial memory leaks
  builtin/credential-store: fix leaking credential
  builtin/show-branch: fix several memory leaks
  builtin/rev-parse: fix memory leak with `--parseopt`
  builtin/stash: fix various trivial memory leaks
  builtin/remote: fix various trivial memory leaks
  builtin/remote: fix leaking strings in `branch_list`
  builtin/ls-remote: fix leaking `pattern` strings
  builtin/submodule--helper: fix leaking buffer in `is_tip_reachable`
  builtin/submodule--helper: fix leaking clone depth parameter
  builtin/name-rev: fix various trivial memory leaks
  builtin/describe: fix trivial memory leak when describing blob
  ...

Author: gitster

builtin/credential-cache.c

@@ -88,6 +88,8 @@ static void spawn_daemon(const char *socket)
 		die_errno("unable to read result code from cache daemon");
 	if (r != 3 || memcmp(buf, "ok\n", 3))
 		die("cache daemon did not start: %.*s", r, buf);
+
+	child_process_clear(&daemon);
 	close(daemon.out);
 }
 
@@ -137,7 +139,8 @@ static void announce_capabilities(void)
 
 int cmd_credential_cache(int argc, const char **argv, const char *prefix)
 {
-	char *socket_path = NULL;
+	const char *socket_path_arg = NULL;
+	char *socket_path;
 	int timeout = 900;
 	const char *op;
 	const char * const usage[] = {
@@ -147,7 +150,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)
 	struct option options[] = {
 		OPT_INTEGER(0, "timeout", &timeout,
 			    "number of seconds to cache credentials"),
-		OPT_STRING(0, "socket", &socket_path, "path",
+		OPT_STRING(0, "socket", &socket_path_arg, "path",
 			   "path of cache-daemon socket"),
 		OPT_END()
 	};
@@ -160,6 +163,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)
 	if (!have_unix_sockets())
 		die(_("credential-cache unavailable; no unix socket support"));
 
+	socket_path = xstrdup_or_null(socket_path_arg);
 	if (!socket_path)
 		socket_path = get_socket_path();
 	if (!socket_path)
@@ -176,6 +180,7 @@ int cmd_credential_cache(int argc, const char **argv, const char *prefix)
 	else
 		; /* ignore unknown operation */
 
+	free(socket_path);
 	return 0;
 }
 

builtin/credential-store.c

@@ -218,5 +218,6 @@ int cmd_credential_store(int argc, const char **argv, const char *prefix)
 		; /* Ignore unknown operation. */
 
 	string_list_clear(&fns, 0);
+	credential_clear(&c);
 	return 0;
 }

builtin/describe.c

@@ -529,6 +529,7 @@ static void describe_blob(struct object_id oid, struct strbuf *dst)
 	traverse_commit_list(&revs, process_commit, process_object, &pcd);
 	reset_revision_walk();
 	release_revisions(&revs);
+	strvec_clear(&args);
 }
 
 static void describe(const char *arg, int last_one)
@@ -619,6 +620,8 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 	if (contains) {
 		struct string_list_item *item;
 		struct strvec args;
+		const char **argv_copy;
+		int ret;
 
 		strvec_init(&args);
 		strvec_pushl(&args, "name-rev",
@@ -637,7 +640,21 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			strvec_pushv(&args, argv);
 		else
 			strvec_push(&args, "HEAD");
-		return cmd_name_rev(args.nr, args.v, prefix);
+
+		/*
+		 * `cmd_name_rev()` modifies the array, so we'd leak its
+		 * contained strings if we didn't do a copy here.
+		 */
+		ALLOC_ARRAY(argv_copy, args.nr + 1);
+		for (size_t i = 0; i < args.nr; i++)
+			argv_copy[i] = args.v[i];
+		argv_copy[args.nr] = NULL;
+
+		ret = cmd_name_rev(args.nr, argv_copy, prefix);
+
+		strvec_clear(&args);
+		free(argv_copy);
+		return ret;
 	}
 
 	hashmap_init(&names, commit_name_neq, NULL, 0);
@@ -679,7 +696,6 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 		} else if (dirty) {
 			struct lock_file index_lock = LOCK_INIT;
 			struct rev_info revs;
-			struct strvec args = STRVEC_INIT;
 			int fd;
 
 			setup_work_tree();
@@ -694,8 +710,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 				repo_update_index_if_able(the_repository, &index_lock);
 
 			repo_init_revisions(the_repository, &revs, prefix);
-			strvec_pushv(&args, diff_index_args);
-			if (setup_revisions(args.nr, args.v, &revs, NULL) != 1)
+
+			if (setup_revisions(ARRAY_SIZE(diff_index_args) - 1,
+					    diff_index_args, &revs, NULL) != 1)
 				BUG("malformed internal diff-index command line");
 			run_diff_index(&revs, 0);
 

builtin/log.c

@@ -1434,6 +1434,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	int need_8bit_cte = 0;
 	struct pretty_print_context pp = {0};
 	struct commit *head = list[0];
+	char *to_free = NULL;
 
 	if (!cmit_fmt_is_mail(rev->commit_format))
 		die(_("cover letter needs email format"));
@@ -1455,7 +1456,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 	}
 
 	if (!branch_name)
-		branch_name = find_branch_name(rev);
+		branch_name = to_free = find_branch_name(rev);
 
 	pp.fmt = CMIT_FMT_EMAIL;
 	pp.date_mode.type = DATE_RFC2822;
@@ -1466,6 +1467,7 @@ static void make_cover_letter(struct rev_info *rev, int use_separate_file,
 			   encoding, need_8bit_cte, cfg);
 	fprintf(rev->diffopt.file, "%s\n", sb.buf);
 
+	free(to_free);
 	free(pp.after_subject);
 	strbuf_release(&sb);
 

builtin/ls-remote.c

@@ -19,17 +19,16 @@ static const char * const ls_remote_usage[] = {
  * Is there one among the list of patterns that match the tail part
  * of the path?
  */
-static int tail_match(const char **pattern, const char *path)
+static int tail_match(const struct strvec *pattern, const char *path)
 {
-	const char *p;
 	char *pathbuf;
 
-	if (!pattern)
+	if (!pattern->nr)
 		return 1; /* no restriction */
 
 	pathbuf = xstrfmt("/%s", path);
-	while ((p = *(pattern++)) != NULL) {
-		if (!wildmatch(p, pathbuf, 0)) {
+	for (size_t i = 0; i < pattern->nr; i++) {
+		if (!wildmatch(pattern->v[i], pathbuf, 0)) {
 			free(pathbuf);
 			return 1;
 		}
@@ -47,7 +46,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	int status = 0;
 	int show_symref_target = 0;
 	const char *uploadpack = NULL;
-	const char **pattern = NULL;
+	struct strvec pattern = STRVEC_INIT;
 	struct transport_ls_refs_options transport_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
 	int i;
@@ -93,13 +92,8 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 
 	packet_trace_identity("ls-remote");
 
-	if (argc > 1) {
-		int i;
-		CALLOC_ARRAY(pattern, argc);
-		for (i = 1; i < argc; i++) {
-			pattern[i - 1] = xstrfmt("*/%s", argv[i]);
-		}
-	}
+	for (int i = 1; i < argc; i++)
+		strvec_pushf(&pattern, "*/%s", argv[i]);
 
 	if (flags & REF_TAGS)
 		strvec_push(&transport_options.ref_prefixes, "refs/tags/");
@@ -136,7 +130,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		struct ref_array_item *item;
 		if (!check_ref_type(ref, flags))
 			continue;
-		if (!tail_match(pattern, ref->name))
+		if (!tail_match(&pattern, ref->name))
 			continue;
 		item = ref_array_push(&ref_array, ref->name, &ref->old_oid);
 		item->symref = xstrdup_or_null(ref->symref);
@@ -158,5 +152,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	if (transport_disconnect(transport))
 		status = 1;
 	transport_ls_refs_options_release(&transport_options);
+
+	strvec_clear(&pattern);
 	return status;
 }

builtin/name-rev.c

@@ -677,7 +677,9 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 				  always, allow_undefined, data.name_only);
 	}
 
-	UNLEAK(string_pool);
-	UNLEAK(revs);
+	string_list_clear(&data.ref_filters, 0);
+	string_list_clear(&data.exclude_filters, 0);
+	mem_pool_discard(&string_pool, 0);
+	object_array_clear(&revs);
 	return 0;
 }

builtin/remote.c

@@ -258,7 +258,7 @@ struct branch_info {
 	char *push_remote_name;
 };
 
-static struct string_list branch_list = STRING_LIST_INIT_NODUP;
+static struct string_list branch_list = STRING_LIST_INIT_DUP;
 
 static const char *abbrev_ref(const char *name, const char *prefix)
 {
@@ -292,8 +292,8 @@ static int config_read_branches(const char *key, const char *value,
 		type = PUSH_REMOTE;
 	else
 		return 0;
-	name = xmemdupz(key, key_len);
 
+	name = xmemdupz(key, key_len);
 	item = string_list_insert(&branch_list, name);
 
 	if (!item->util)
@@ -337,6 +337,7 @@ static int config_read_branches(const char *key, const char *value,
 		BUG("unexpected type=%d", type);
 	}
 
+	free(name);
 	return 0;
 }
 
@@ -554,13 +555,16 @@ static int add_branch_for_removal(const char *refname,
 	refspec.dst = (char *)refname;
 	if (remote_find_tracking(branches->remote, &refspec))
 		return 0;
+	free(refspec.src);
 
 	/* don't delete a branch if another remote also uses it */
 	for (kr = branches->keep->list; kr; kr = kr->next) {
 		memset(&refspec, 0, sizeof(refspec));
 		refspec.dst = (char *)refname;
-		if (!remote_find_tracking(kr->remote, &refspec))
+		if (!remote_find_tracking(kr->remote, &refspec)) {
+			free(refspec.src);
 			return 0;
+		}
 	}
 
 	/* don't delete non-remote-tracking refs */
@@ -667,7 +671,11 @@ static int config_read_push_default(const char *key, const char *value,
 static void handle_push_default(const char* old_name, const char* new_name)
 {
 	struct push_default_info push_default = {
-		old_name, CONFIG_SCOPE_UNKNOWN, STRBUF_INIT, -1 };
+		.old_name = old_name,
+		.scope = CONFIG_SCOPE_UNKNOWN,
+		.origin = STRBUF_INIT,
+		.linenr = -1,
+	};
 	git_config(config_read_push_default, &push_default);
 	if (push_default.scope >= CONFIG_SCOPE_COMMAND)
 		; /* pass */
@@ -687,6 +695,8 @@ static void handle_push_default(const char* old_name, const char* new_name)
 			push_default.origin.buf, push_default.linenr,
 			old_name);
 	}
+
+	strbuf_release(&push_default.origin);
 }
 
 
@@ -784,7 +794,7 @@ static int mv(int argc, const char **argv, const char *prefix)
 	}
 
 	if (!refspec_updated)
-		return 0;
+		goto out;
 
 	/*
 	 * First remove symrefs, then rename the rest, finally create
@@ -850,10 +860,15 @@ static int mv(int argc, const char **argv, const char *prefix)
 		display_progress(progress, ++refs_renamed_nr);
 	}
 	stop_progress(&progress);
-	string_list_clear(&remote_branches, 1);
 
 	handle_push_default(rename.old_name, rename.new_name);
 
+out:
+	string_list_clear(&remote_branches, 1);
+	strbuf_release(&old_remote_context);
+	strbuf_release(&buf);
+	strbuf_release(&buf2);
+	strbuf_release(&buf3);
 	return 0;
 }
 
@@ -944,12 +959,21 @@ static int rm(int argc, const char **argv, const char *prefix)
 
 	if (!result) {
 		strbuf_addf(&buf, "remote.%s", remote->name);
-		if (git_config_rename_section(buf.buf, NULL) < 1)
-			return error(_("Could not remove config section '%s'"), buf.buf);
+		if (git_config_rename_section(buf.buf, NULL) < 1) {
+			result = error(_("Could not remove config section '%s'"), buf.buf);
+			goto out;
+		}
 
 		handle_push_default(remote->name, NULL);
 	}
 
+out:
+	for (struct known_remote *r = known_remotes.list; r;) {
+		struct known_remote *next = r->next;
+		free(r);
+		r = next;
+	}
+	strbuf_release(&buf);
 	return result;
 }
 
@@ -982,8 +1006,10 @@ static int append_ref_to_tracked_list(const char *refname,
 
 	memset(&refspec, 0, sizeof(refspec));
 	refspec.dst = (char *)refname;
-	if (!remote_find_tracking(states->remote, &refspec))
+	if (!remote_find_tracking(states->remote, &refspec)) {
 		string_list_append(&states->tracked, abbrev_branch(refspec.src));
+		free(refspec.src);
+	}
 
 	return 0;
 }