Thread (24 messages) flat view 24 messages, 3 authors, 2016-07-10

Re: [PATCH] am: ignore return value of write_file()

From: Jeff King <hidden>
Date: 2016-07-08 21:51:21

On Fri, Jul 08, 2016 at 08:44:28PM +0200, René Scharfe wrote:
quoted
The question is whether it makes sense for write_file() to die(). It is a
library function and not every caller can be happy with that function to
exit the program when some file could not be written, without a chance to
tell the user what to do about the situation.

If write_file() was defined in builtin/am.c, as a static function, I would
grudgingly acquiesce, but it is not.

IMO it would be better to fix write_file() to *not* die() but return
error() instead.
there is write_file_gently() for that purpose, but it's used only by a
single caller that exits on failure after all, and in fact Peff's series
drops it.
Yeah, I always feel funny going in the opposite direction of
libification, as in this case. But having looked at the set of current
and potential-to-convert callers, I couldn't find a single one which
would want the gentle behavior. Any site which doesn't die also wanted
something else more complex (e.g., different open options).

So I think rather than loading down write_file() with options that will
make the simple callers harder to read, we are better off to keep its
implementation simple, and let people call its building blocks easily.

And I think we already have that; in my final version it really is just
xopen/write_in_full/close. So the non-simple sites can still make use of
those components.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help