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

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jun 29, 2026 at 09:08:42AM -0700, Junio C Hamano wrote:
[snip]
>  * Changes from v1 are two additional error checks to notice failure
>    from fwrite() and fclose() to die.  Interdiff appears at the end.

Technically speaking the first error check for fwrite() should be
unnecessary as the errors accumulate. But it doesn't hurt, either.

> Interdiff against v1:
>   diff --git a/builtin/history.c b/builtin/history.c
>   index f17ec049c0..365e81379b 100644
>   --- a/builtin/history.c
>   +++ b/builtin/history.c
>   @@ -59,13 +59,15 @@ static int fill_commit_message(struct repository *repo,
>    	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);
>   +	if (fwrite(out->buf, 1, out->len, s.fp) != out->len)
>   +		die_errno(_("could not write to '%s'"), path);
>    
>    	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);
>   +	if (fclose(s.fp))
>   +		die_errno(_("could not write to '%s'"), path);
>    
>    	strbuf_reset(out);
>    	if (launch_editor(path, out, NULL)) {

Yup, this looks good to me. Thanks!

Patrick