Re: [PATCH 1/4] error: save and restore errno

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 19, 2014, 18:14 UTC
Message-ID: <xmqqvbmbrrba.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20141119014722.GB2337@peff.net>

Jeff King <peff@peff.net> writes:

Show 34 quoted lines

> On Tue, Nov 18, 2014 at 05:43:44PM -0800, Jonathan Nieder wrote:
>
>> Jeff King wrote:
>> 
>> > It's common to use error() to return from a function, like:
>> >
>> > 	if (open(...) < 0)
>> > 		return error("open failed");
>> >
>> > Unfortunately this may clobber the errno from the open()
>> > call. So we often end up with code like this:
>> >
>> >         if (open(...) < 0) {
>> > 		int saved_errno = errno;
>> > 		error("open failed");
>> > 		errno = saved_errno;
>> > 		return -1;
>> > 	}
>> >
>> > which is less nice.
>> 
>> What the above doesn't explain is why the caller cares about errno.
>> Are they going to print another message with strerror(errno)?  Or are
>> they going to consider some errors non-errors (like ENOENT when trying
>> to unlink a file), in which case why is printing a message to stderr
>> okay?
>
> I guess the unsaid bit is:
>
>   Unfortunately this may clobber the errno from the open() call. Even
>   though error() sees the correct errno, the caller to which we are
>   returning may see a bogus errno value.
>
> -Peff

I am not sure if that answers the question asked.

If you have

	int frotz(...) {
		int fd = open(...);
        	if (fd < 0)
                	return error("open failed (%s)", strerror(errno));
		return fd;
	}

and the caller calls it and cares about the errno from this open, what does the caller do? Jonathan's worried about a codepath that may be familiar to us as we recently saw a patch similar to it:

	int fd = frotz(...);
        if (fd < 0) {
        	if (errno == ENOENT || errno == EISDIR)
                	; /* not quite an error */
		else
			exit(1);
	}

If ENOENT/EISDIR is expected and a non-error, it is not useful for frotz() to give an error message on its own.

I think a more appropriate answer to Jonathan's question is why is the callee (i.e. frotz()) calling error() in the first place if an unconditional error message is an issue.