Re: [PATCH 1/2] sequencer: factor out rewrite_file()

From: René Scharfe <l.s.r@web.de>
Date: Nov 4, 2017, 09:05 UTC
Message-ID: <b074d6fa-8778-ea0d-d53b-7cc35bc4264a@web.de>
In-Reply-To: <20171103191309.sth4zjokgcupvk2e@sigill.intra.peff.net>

Am 03.11.2017 um 20:13 schrieb Jeff King:

Show 18 quoted lines

> On Fri, Nov 03, 2017 at 10:44:08PM +0900, Junio C Hamano wrote:
> 
>> Simon Ruderich <simon@ruderich.org> writes:
>>
>>> I tried looking into this by adding a new write_file_buf_gently()
>>> (or maybe renaming write_file_buf to write_file_buf_or_die) and
>>> using it from write_file_buf() but I don't know the proper way to
>>> handle the error-case in write_file_buf(). Just calling
>>> die("write_file_buf") feels ugly, as the real error was already
>>> printed on screen by error_errno() and I didn't find any function
>>> to just exit without writing a message (which still respects
>>> die_routine). Suggestions welcome.
>>
>> How about *not* printing the error at the place where you notice the
>> error, and instead return an error code to the caller to be noticed
>> which dies with an error message?
> 
> That ends up giving less-specific errors.

Not necessarily. Function could return different codes for different errors, e.g. -1 for an open(2) error and -2 for a write(2) error, and the caller could use that to select the message to show.

Basically all of the messages in wrapper.c consist of some text mixed with the affected path path and a strerror(3) string, so they're compatible in that way. A single function (get_path_error_format()?) could thus be used and callers would be able to combine its result with die(), error(), or warning().

René