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

[Patrick Steinhardt <ps@pks.im>]

On Mon, Jun 29, 2026 at 08:21:06AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> +	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.
> 
> Yes, the fwrite() should be protected with an error checking and
> die() the same way as the code before.  Will send a v2.
> 
> But isn't the end result the same between preimage and postimage?
> If the stuff appended by wt_status_* are still written without error
> checking, we would leave a partially-written file that has the
> default_messages and the commented hint/action but not necessarily
> whatever we wanted to add with wt_status().

At least it would only be the status information that's missing in that
case, the commit message itself would be retained (or we'd die if it
wasn't written). So we didn't have the potential to loose information
that is intended to end up in the final commit.

Patrick