Merge branch 'rs/strbuf-leakfix'

gitster · gitster · commit d811ba1897b7 · 2017-09-19T10:47:57.000+09:00
Many leaks of strbuf have been fixed.

* rs/strbuf-leakfix: (34 commits)
  wt-status: release strbuf after use in wt_longstatus_print_tracking()
  wt-status: release strbuf after use in read_rebase_todolist()
  vcs-svn: release strbuf after use in end_revision()
  utf8: release strbuf on error return in strbuf_utf8_replace()
  userdiff: release strbuf after use in userdiff_get_textconv()
  transport-helper: release strbuf after use in process_connect_service()
  sequencer: release strbuf after use in save_head()
  shortlog: release strbuf after use in insert_one_record()
  sha1_file: release strbuf on error return in index_path()
  send-pack: release strbuf on error return in send_pack()
  remote: release strbuf after use in set_url()
  remote: release strbuf after use in migrate_file()
  remote: release strbuf after use in read_remote_branches()
  refs: release strbuf on error return in write_pseudoref()
  notes: release strbuf after use in notes_copy_from_stdin()
  merge: release strbuf after use in write_merge_heads()
  merge: release strbuf after use in save_state()
  mailinfo: release strbuf on error return in handle_boundary()
  mailinfo: release strbuf after use in handle_from()
  help: release strbuf on error return in exec_woman_emacs()
  ...
diff --git a/builtin/am.c b/builtin/am.c
@@ -671,9 +671,7 @@ static int detect_patch_format(const char **paths)
 		goto done;
 	}
 
-	strbuf_reset(&l2);
 	strbuf_getline(&l2, fp);
-	strbuf_reset(&l3);
 	strbuf_getline(&l3, fp);
 
 	/*
@@ -696,6 +694,8 @@ static int detect_patch_format(const char **paths)
 done:
 	fclose(fp);
 	strbuf_release(&l1);
+	strbuf_release(&l2);
+	strbuf_release(&l3);
 	return ret;
 }
 
@@ -881,6 +881,7 @@ static int split_mail_stgit_series(struct am_state *state, const char **paths,
 static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr)
 {
 	struct strbuf sb = STRBUF_INIT;
+	int rc = 0;
 
 	while (!strbuf_getline_lf(&sb, in)) {
 		const char *str;
@@ -894,19 +895,27 @@ static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr)
 
 			errno = 0;
 			timestamp = parse_timestamp(str, &end, 10);
-			if (errno)
-				return error(_("invalid timestamp"));
+			if (errno) {
+				rc = error(_("invalid timestamp"));
+				goto exit;
+			}
 
-			if (!skip_prefix(end, " ", &str))
-				return error(_("invalid Date line"));
+			if (!skip_prefix(end, " ", &str)) {
+				rc = error(_("invalid Date line"));
+				goto exit;
+			}
 
 			errno = 0;
 			tz = strtol(str, &end, 10);
-			if (errno)
-				return error(_("invalid timezone offset"));
+			if (errno) {
+				rc = error(_("invalid timezone offset"));
+				goto exit;
+			}
 
-			if (*end)
-				return error(_("invalid Date line"));
+			if (*end) {
+				rc = error(_("invalid Date line"));
+				goto exit;
+			}
 
 			/*
 			 * mercurial's timezone is in seconds west of UTC,
@@ -931,9 +940,9 @@ static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr)
 		fwrite(sb.buf, 1, sb.len, out);
 		strbuf_reset(&sb);
 	}
-
+exit:
 	strbuf_release(&sb);
-	return 0;
+	return rc;
 }
 
 /**
@@ -2096,6 +2105,7 @@ static int safe_to_abort(const struct am_state *state)
 			die(_("could not parse %s"), am_path(state, "abort-safety"));
 	} else
 		oidclr(&abort_safety);
+	strbuf_release(&sb);
 
 	if (get_oid("HEAD", &head))
 		oidclr(&head);
diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
@@ -45,6 +45,7 @@ static int check_ref_format_branch(const char *arg)
 	if (strbuf_check_branch_ref(&sb, arg))
 		die("'%s' is not a valid branch name", arg);
 	printf("%s\n", sb.buf + 11);
+	strbuf_release(&sb);
 	return 0;
 }
 
diff --git a/builtin/clean.c b/builtin/clean.c
@@ -167,7 +167,7 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		}
 
 		*dir_gone = 0;
-		return 0;
+		goto out;
 	}
 
 	dir = opendir(path->buf);
@@ -181,7 +181,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 			warning_errno(_(msg_warn_remove_failed), quoted.buf);
 			*dir_gone = 0;
 		}
-		return res;
+		ret = res;
+		goto out;
 	}
 
 	strbuf_complete(path, '/');
@@ -249,6 +250,8 @@ static int remove_dirs(struct strbuf *path, const char *prefix, int force_flag,
 		for (i = 0; i < dels.nr; i++)
 			printf(dry_run ?  _(msg_would_remove) : _(msg_remove), dels.items[i].string);
 	}
+out:
+	strbuf_release(&quoted);
 	string_list_clear(&dels, 0);
 	return ret;
 }
diff --git a/builtin/clone.c b/builtin/clone.c
@@ -507,8 +507,8 @@ static void remove_junk(void)
 	if (junk_work_tree) {
 		strbuf_addstr(&sb, junk_work_tree);
 		remove_dir_recursively(&sb, 0);
-		strbuf_reset(&sb);
 	}
+	strbuf_release(&sb);
 }
 
 static void remove_junk_on_signal(int signo)
diff --git a/builtin/help.c b/builtin/help.c
@@ -131,6 +131,7 @@ static void exec_woman_emacs(const char *path, const char *page)
 		strbuf_addf(&man_page, "(woman \"%s\")", page);
 		execlp(path, "emacsclient", "-e", man_page.buf, (char *)NULL);
 		warning_errno(_("failed to exec '%s'"), path);
+		strbuf_release(&man_page);
 	}
 }
 
@@ -152,6 +153,7 @@ static void exec_man_konqueror(const char *path, const char *page)
 		strbuf_addf(&man_page, "man:%s(1)", page);
 		execlp(path, filename, "newTab", man_page.buf, (char *)NULL);
 		warning_errno(_("failed to exec '%s'"), path);
+		strbuf_release(&man_page);
 	}
 }
 
@@ -169,6 +171,7 @@ static void exec_man_cmd(const char *cmd, const char *page)
 	strbuf_addf(&shell_cmd, "%s %s", cmd, page);
 	execl(SHELL_PATH, SHELL_PATH, "-c", shell_cmd.buf, (char *)NULL);
 	warning(_("failed to exec '%s'"), cmd);
+	strbuf_release(&shell_cmd);
 }
 
 static void add_man_viewer(const char *name)
diff --git a/builtin/merge.c b/builtin/merge.c
@@ -255,6 +255,7 @@ static int save_state(struct object_id *stash)
 	struct child_process cp = CHILD_PROCESS_INIT;
 	struct strbuf buffer = STRBUF_INIT;
 	const char *argv[] = {"stash", "create", NULL};
+	int rc = -1;
 
 	cp.argv = argv;
 	cp.out = -1;
@@ -268,11 +269,14 @@ static int save_state(struct object_id *stash)
 	if (finish_command(&cp) || len < 0)
 		die(_("stash failed"));
 	else if (!len)		/* no changes */
-		return -1;
+		goto out;
 	strbuf_setlen(&buffer, buffer.len-1);
 	if (get_oid(buffer.buf, stash))
 		die(_("not a valid object: %s"), buffer.buf);
-	return 0;
+	rc = 0;
+out:
+	strbuf_release(&buffer);
+	return rc;
 }
 
 static void read_empty(unsigned const char *sha1, int verbose)
@@ -942,6 +946,7 @@ static void write_merge_heads(struct commit_list *remoteheads)
 	if (fast_forward == FF_NO)
 		strbuf_addstr(&buf, "no-ff");
 	write_file_buf(git_path_merge_mode(), buf.buf, buf.len);
+	strbuf_release(&buf);
 }
 
 static void write_merge_state(struct commit_list *remoteheads)
diff --git a/builtin/notes.c b/builtin/notes.c
@@ -328,6 +328,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
 	} else {
 		finish_copy_notes_for_rewrite(c, msg);
 	}
+	strbuf_release(&buf);
 	return ret;
 }
 
diff --git a/builtin/remote.c b/builtin/remote.c
@@ -571,6 +571,7 @@ static int read_remote_branches(const char *refname,
 		else
 			item->util = NULL;
 	}
+	strbuf_release(&buf);
 
 	return 0;
 }
@@ -595,6 +596,7 @@ static int migrate_file(struct remote *remote)
 		unlink_or_warn(git_path("remotes/%s", remote->name));
 	else if (remote->origin == REMOTE_BRANCHES)
 		unlink_or_warn(git_path("branches/%s", remote->name));
+	strbuf_release(&buf);
 
 	return 0;
 }
@@ -1563,9 +1565,7 @@ static int set_url(int argc, const char **argv)
 						       "^$", 0);
 		else
 			git_config_set(name_buf.buf, newurl);
-		strbuf_release(&name_buf);
-
-		return 0;
+		goto out;
 	}
 
 	/* Old URL specified. Demand that one matches. */
@@ -1588,6 +1588,8 @@ static int set_url(int argc, const char **argv)
 		git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);
 	else
 		git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);
+out:
+	strbuf_release(&name_buf);
 	return 0;
 }
 
diff --git a/commit.c b/commit.c
@@ -1570,10 +1570,13 @@ int commit_tree_extended(const char *msg, size_t msg_len,
 	if (encoding_is_utf8 && !verify_utf8(&buffer))
 		fprintf(stderr, _(commit_utf8_warn));
 
-	if (sign_commit && do_sign_commit(&buffer, sign_commit))
-		return -1;
+	if (sign_commit && do_sign_commit(&buffer, sign_commit)) {
+		result = -1;
+		goto out;
+	}
 
 	result = write_sha1_file(buffer.buf, buffer.len, commit_type, ret);
+out:
 	strbuf_release(&buffer);
 	return result;
 }
diff --git a/connect.c b/connect.c
@@ -778,7 +778,6 @@ struct child_process *git_connect(int fd[2], const char *url,
 	char *hostandport, *path;
 	struct child_process *conn = &no_fork;
 	enum protocol protocol;
-	struct strbuf cmd = STRBUF_INIT;
 
 	/* Without this we cannot rely on waitpid() to tell
 	 * what happened to our children.
@@ -826,6 +825,8 @@ struct child_process *git_connect(int fd[2], const char *url,
 			     target_host, 0);
 		free(target_host);
 	} else {
+		struct strbuf cmd = STRBUF_INIT;
+
 		conn = xmalloc(sizeof(*conn));
 		child_process_init(conn);
 
@@ -862,6 +863,7 @@ struct child_process *git_connect(int fd[2], const char *url,
 				free(hostandport);
 				free(path);
 				free(conn);
+				strbuf_release(&cmd);
 				return NULL;
 			}
 
diff --git a/convert.c b/convert.c
@@ -423,8 +423,10 @@ static int filter_buffer_or_fd(int in, int out, void *data)
 	child_process.in = -1;
 	child_process.out = out;
 
-	if (start_command(&child_process))
+	if (start_command(&child_process)) {
+		strbuf_release(&cmd);
 		return error("cannot fork to run external filter '%s'", params->cmd);
+	}
 
 	sigchain_push(SIGPIPE, SIG_IGN);
 
diff --git a/diff.c b/diff.c
@@ -2583,6 +2583,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options)
 	}
 
 	print_stat_summary_inserts_deletes(options, total_files, adds, dels);
+	strbuf_release(&out);
 }
 
 static void show_shortstats(struct diffstat_t *data, struct diff_options *options)
@@ -5288,6 +5289,7 @@ static void show_rename_copy(struct diff_options *opt, const char *renamecopy,
 	emit_diff_symbol(opt, DIFF_SYMBOL_SUMMARY,
 				 sb.buf, sb.len, 0);
 	show_mode_change(opt, p, 0);
+	strbuf_release(&sb);
 }
 
 static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
@@ -5313,6 +5315,7 @@ static void diff_summary(struct diff_options *opt, struct diff_filepair *p)
 			strbuf_addf(&sb, " (%d%%)\n", similarity_index(p));
 			emit_diff_symbol(opt, DIFF_SYMBOL_SUMMARY,
 					 sb.buf, sb.len, 0);
+			strbuf_release(&sb);
 		}
 		show_mode_change(opt, p, !p->score);
 		break;
diff --git a/mailinfo.c b/mailinfo.c
@@ -149,16 +149,14 @@ static void handle_from(struct mailinfo *mi, const struct strbuf *from)
 	at = strchr(f.buf, '@');
 	if (!at) {
 		parse_bogus_from(mi, from);
-		return;
+		goto out;
 	}
 
 	/*
 	 * If we already have one email, don't take any confusing lines
 	 */
-	if (mi->email.len && strchr(at + 1, '@')) {
-		strbuf_release(&f);
-		return;
-	}
+	if (mi->email.len && strchr(at + 1, '@'))
+		goto out;
 
 	/* Pick up the string around '@', possibly delimited with <>
 	 * pair; that is the email part.
@@ -198,6 +196,7 @@ static void handle_from(struct mailinfo *mi, const struct strbuf *from)
 	}
 
 	get_sane_name(&mi->name, &f, &mi->email);
+out:
 	strbuf_release(&f);
 }
 
@@ -929,6 +928,7 @@ static int handle_boundary(struct mailinfo *mi, struct strbuf *line)
 			error("Detected mismatched boundaries, can't recover");
 			mi->input_error = -1;
 			mi->content_top = mi->content;
+			strbuf_release(&newline);
 			return 0;
 		}
 		handle_filter(mi, &newline);
diff --git a/refs.c b/refs.c
@@ -594,7 +594,7 @@ static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,
 	if (fd < 0) {
 		strbuf_addf(err, "could not open '%s' for writing: %s",
 			    filename, strerror(errno));
-		return -1;
+		goto done;
 	}
 
 	if (old_sha1) {
diff --git a/send-pack.c b/send-pack.c
@@ -492,8 +492,11 @@ int send_pack(struct send_pack_args *args,
 			 * we were to send it and we're trying to send the refs
 			 * atomically, abort the whole operation.
 			 */
-			if (use_atomic)
+			if (use_atomic) {
+				strbuf_release(&req_buf);
+				strbuf_release(&cap_buf);
 				return atomic_push_failure(args, remote_refs, ref);
+			}
 			/* Fallthrough for non atomic case. */
 		default:
 			continue;
diff --git a/sequencer.c b/sequencer.c
@@ -1565,14 +1565,17 @@ static int save_head(const char *head)
 	static struct lock_file head_lock;
 	struct strbuf buf = STRBUF_INIT;
 	int fd;
+	ssize_t written;
 
 	fd = hold_lock_file_for_update(&head_lock, git_path_head_file(), 0);
 	if (fd < 0) {
 		rollback_lock_file(&head_lock);
 		return error_errno(_("could not lock HEAD"));
 	}
 	strbuf_addf(&buf, "%s\n", head);
-	if (write_in_full(fd, buf.buf, buf.len) < 0) {
+	written = write_in_full(fd, buf.buf, buf.len);
+	strbuf_release(&buf);
+	if (written < 0) {
 		rollback_lock_file(&head_lock);
 		return error_errno(_("could not write to '%s'"),
 				   git_path_head_file());
diff --git a/sha1_file.c b/sha1_file.c
diff --git a/transport-helper.c b/transport-helper.c
diff --git a/userdiff.c b/userdiff.c
diff --git a/utf8.c b/utf8.c
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
diff --git a/wt-status.c b/wt-status.c

Original file line number	Diff line number	Diff line change
`@@ -45,6 +45,7 @@ static int check_ref_format_branch(const char *arg)`
`45`	`45`	`if (strbuf_check_branch_ref(&sb, arg))`
`46`	`46`	`die("'%s' is not a valid branch name", arg);`
`47`	`47`	`printf("%s\n", sb.buf + 11);`
	`48`	`+ strbuf_release(&sb);`
`48`	`49`	`return 0;`
`49`	`50`	`}`
`50`	`51`
Original file line number	Diff line number	Diff line change
`@@ -167,7 +167,7 @@ static int remove_dirs(struct strbuf path, const char prefix, int force_flag,`
`167`	`167`	`}`
`168`	`168`
`169`	`169`	`*dir_gone = 0;`
`170`		`- return 0;`
	`170`	`+ goto out;`
`171`	`171`	`}`
`172`	`172`
`173`	`173`	`dir = opendir(path->buf);`
`@@ -181,7 +181,8 @@ static int remove_dirs(struct strbuf path, const char prefix, int force_flag,`
`181`	`181`	`warning_errno(_(msg_warn_remove_failed), quoted.buf);`
`182`	`182`	`*dir_gone = 0;`
`183`	`183`	`}`
`184`		`- return res;`
	`184`	`+ ret = res;`
	`185`	`+ goto out;`
`185`	`186`	`}`
`186`	`187`
`187`	`188`	`strbuf_complete(path, '/');`
`@@ -249,6 +250,8 @@ static int remove_dirs(struct strbuf path, const char prefix, int force_flag,`
`249`	`250`	`for (i = 0; i < dels.nr; i++)`
`250`	`251`	`printf(dry_run ? _(msg_would_remove) : _(msg_remove), dels.items[i].string);`
`251`	`252`	`}`
	`253`	`+out:`
	`254`	`+ strbuf_release(&quoted);`
`252`	`255`	`string_list_clear(&dels, 0);`
`253`	`256`	`return ret;`
`254`	`257`	`}`
Original file line number	Diff line number	Diff line change
`@@ -507,8 +507,8 @@ static void remove_junk(void)`
`507`	`507`	`if (junk_work_tree) {`
`508`	`508`	`strbuf_addstr(&sb, junk_work_tree);`
`509`	`509`	`remove_dir_recursively(&sb, 0);`
`510`		`- strbuf_reset(&sb);`
`511`	`510`	`}`
	`511`	`+ strbuf_release(&sb);`
`512`	`512`	`}`
`513`	`513`
`514`	`514`	`static void remove_junk_on_signal(int signo)`
Original file line number	Diff line number	Diff line change
`@@ -131,6 +131,7 @@ static void exec_woman_emacs(const char path, const char page)`
`131`	`131`	`strbuf_addf(&man_page, "(woman \"%s\")", page);`
`132`	`132`	`execlp(path, "emacsclient", "-e", man_page.buf, (char *)NULL);`
`133`	`133`	`warning_errno(_("failed to exec '%s'"), path);`
	`134`	`+ strbuf_release(&man_page);`
`134`	`135`	`}`
`135`	`136`	`}`
`136`	`137`
`@@ -152,6 +153,7 @@ static void exec_man_konqueror(const char path, const char page)`
`152`	`153`	`strbuf_addf(&man_page, "man:%s(1)", page);`
`153`	`154`	`execlp(path, filename, "newTab", man_page.buf, (char *)NULL);`
`154`	`155`	`warning_errno(_("failed to exec '%s'"), path);`
	`156`	`+ strbuf_release(&man_page);`
`155`	`157`	`}`
`156`	`158`	`}`
`157`	`159`
`@@ -169,6 +171,7 @@ static void exec_man_cmd(const char cmd, const char page)`
`169`	`171`	`strbuf_addf(&shell_cmd, "%s %s", cmd, page);`
`170`	`172`	`execl(SHELL_PATH, SHELL_PATH, "-c", shell_cmd.buf, (char *)NULL);`
`171`	`173`	`warning(_("failed to exec '%s'"), cmd);`
	`174`	`+ strbuf_release(&shell_cmd);`
`172`	`175`	`}`
`173`	`176`
`174`	`177`	`static void add_man_viewer(const char *name)`
Original file line number	Diff line number	Diff line change
`@@ -328,6 +328,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)`
`328`	`328`	`} else {`
`329`	`329`	`finish_copy_notes_for_rewrite(c, msg);`
`330`	`330`	`}`
	`331`	`+ strbuf_release(&buf);`
`331`	`332`	`return ret;`
`332`	`333`	`}`
`333`	`334`
Original file line number	Diff line number	Diff line change
`@@ -571,6 +571,7 @@ static int read_remote_branches(const char *refname,`
`571`	`571`	`else`
`572`	`572`	`item->util = NULL;`
`573`	`573`	`}`
	`574`	`+ strbuf_release(&buf);`
`574`	`575`
`575`	`576`	`return 0;`
`576`	`577`	`}`
`@@ -595,6 +596,7 @@ static int migrate_file(struct remote *remote)`
`595`	`596`	`unlink_or_warn(git_path("remotes/%s", remote->name));`
`596`	`597`	`else if (remote->origin == REMOTE_BRANCHES)`
`597`	`598`	`unlink_or_warn(git_path("branches/%s", remote->name));`
	`599`	`+ strbuf_release(&buf);`
`598`	`600`
`599`	`601`	`return 0;`
`600`	`602`	`}`
`@@ -1563,9 +1565,7 @@ static int set_url(int argc, const char **argv)`
`1563`	`1565`	`"^$", 0);`
`1564`	`1566`	`else`
`1565`	`1567`	`git_config_set(name_buf.buf, newurl);`
`1566`		`- strbuf_release(&name_buf);`
`1567`		`-`
`1568`		`- return 0;`
	`1568`	`+ goto out;`
`1569`	`1569`	`}`
`1570`	`1570`
`1571`	`1571`	`/* Old URL specified. Demand that one matches. */`
`@@ -1588,6 +1588,8 @@ static int set_url(int argc, const char **argv)`
`1588`	`1588`	`git_config_set_multivar(name_buf.buf, newurl, oldurl, 0);`
`1589`	`1589`	`else`
`1590`	`1590`	`git_config_set_multivar(name_buf.buf, NULL, oldurl, 1);`
	`1591`	`+out:`
	`1592`	`+ strbuf_release(&name_buf);`
`1591`	`1593`	`return 0;`
`1592`	`1594`	`}`
`1593`	`1595`