Re: [PATCH] clean: use warning_errno() when appropriate

From: Jeff King <peff@peff.net>
Date: Feb 13, 2017, 22:22 UTC
Message-ID: <20170213222210.dtfnxbbi77tsx4a6@sigill.intra.peff.net>
In-Reply-To: <xmqqy3x9bvvm.fsf@gitster.mtv.corp.google.com>

On Mon, Feb 13, 2017 at 01:53:33PM -0800, Junio C Hamano wrote:

Show 14 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > IOW, I think this may be a case where we should be optimizing for
> > programmer time (fewer lines of code, and one less thing to worry about
> > in the callers) versus squeezing out every instruction.
> 
> Fair enough.
> 
> Unless we do the save_errno dance in all the helper functions we
> commonly use to safely stash away errno as necessary and tell
> developers that they can depend on it, the code in the patch that
> began this discussion still needs its own saved_errno dance to be
> safe, though.  I do not have a feeling that we are not there yet,
> even after we teach xmalloc() and its family to do so.

Yeah, I certainly agree that is a potential blocker. Even if it is true today, there's nothing guaranteeing that the quote functions don't grow a new internal detail that violates.

So in that sense doing the errno dance as close to the caller who cares is the most _obvious_ thing, even if it isn't the shortest.

It would be nice if there was a way to annotate a function as errno-safe, and then transitively compute which other functions were errno-safe when they do not call any errno-unsafe function. I don't know if any static analyzers allow that kind of custom annotation, though (and also I wonder whether the cost/benefit of maintaining those annotations would be worth it).

-Peff