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

From: Jeff King <peff@peff.net>
Date: Jul 8, 2016, 21:51 UTC
Message-ID: <20160708215114.GC9820@sigill.intra.peff.net>
In-Reply-To: <577FF48C.4030008@web.de>

On Fri, Jul 08, 2016 at 08:44:28PM +0200, René Scharfe wrote:

Show 14 quoted lines

> > 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