Re: [PATCH] history: streamline message preparation and plug file stream leak

[Patrick Steinhardt <ps@pks.im>]

On Fri, Jun 26, 2026 at 09:38:42AM -0700, Junio C Hamano wrote:
> diff --git a/builtin/history.c b/builtin/history.c
> index 8dcb9a6046..f17ec049c0 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -41,11 +41,6 @@ static int fill_commit_message(struct repository *repo,
>  		  " empty message aborts the commit.\n");
>  	struct wt_status s;
>  
> -	strbuf_addstr(out, default_message);
> -	strbuf_addch(out, '\n');
> -	strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
> -	write_file_buf(path, out->buf, out->len);
> -
>  	wt_status_prepare(repo, &s);
>  	FREE_AND_NULL(s.branch);
>  	s.ahead_behind_flags = AHEAD_BEHIND_QUICK;
> @@ -57,14 +52,20 @@ static int fill_commit_message(struct repository *repo,
>  	s.whence = FROM_COMMIT;
>  	s.committable = 1;
>  
> -	s.fp = fopen(git_path_commit_editmsg(), "a");

Here we reuse the local `path` variable, which already carries the
result of `git_path_commit_editmsg()`.

> +	s.fp = fopen(path, "w");
>  	if (!s.fp)
> -		return error_errno(_("could not open '%s'"), git_path_commit_editmsg());
> +		return error_errno(_("could not open '%s'"), path);

Likewise.

> +	strbuf_addstr(out, default_message);
> +	strbuf_addch(out, '\n');
> +	strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str);
> +	fwrite(out->buf, 1, out->len, s.fp);
>  
>  	wt_status_collect_changes_trees(&s, old_tree, new_tree);
>  	wt_status_print(&s);
>  	wt_status_collect_free_buffers(&s);
>  	string_list_clear_func(&s.change, change_data_free);
> +	fclose(s.fp);

This is fixing the leaked file descriptor.

One thing I wonder though is that we don't perform any error checking on
the file in the new version. Previously, we would have died in case
`write_file_buf()` failed. But now we just `fwrite()` without error
checking. I don't think that "wt-status.c" does error checking either,
so we might end up with a partially-written file without us noticing.

Thanks!

Patrick