Re: [GSoC][PATCH v5 02/20] rebase -i: rewrite append_todo_help() in C

Hi Christian
On 07/08/18 17:28, Christian Couder wrote:

On Tue, Aug 7, 2018 at 6:15 PM, Phillip Wood [off-list ref] wrote:

quoted

On 07/08/18 16:25, Christian Couder wrote:

quoted

I agree about checking the return value from fputs(), but it seems to
me that we don't usually check the value of fclose().

A quick grep shows you're right, there are only a handful of places where
the return value of fclose() is checked (there aren't many checks for the
return value of close() either), I'm don't think that is safe though given
that write errors may only show up when the file gets flushed by closing it.

I vaguely remember we tried to check those return values but it didn't
work well sometimes.

I think there's no point in checking the return value after a successful
read, or when writing and the file is being closed (and then possibly
removed) in some clean-up in an error path, but if the code cares about
ensuring the data is written then I think it should be checking the
return value, close_tempfile_gently() does for example.

In the sequencer code there are some examples where it probably should
be checking the return value, and places such as rewrite_file() and
write_message() (which uses the lockfile api) where the return code is
checked (there are a couple of recently added callers of write_message()
in the code to recreate octopus merges that don't check its return value
but all the other callers do).

Best Wishes

Phillip