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

[Junio C Hamano <gitster@pobox.com>]

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().