Re: [PATCH 00/15] Handle fopen() errors

From: Jeff King <peff@peff.net>
Date: Apr 21, 2017, 06:29 UTC
Message-ID: <20170421062915.he5tlgjqq7kj5h32@sigill.intra.peff.net>
In-Reply-To: <xmqqk26e5swj.fsf@gitster.mtv.corp.google.com>

On Thu, Apr 20, 2017 at 08:41:32PM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > I wonder if it is OK to only special case ENOENT for !fp cases,
> > where existing code silently returns.  Perhaps it is trying to read
> > an optional file, and it returns silently because lack of it is
> > perfectly OK for the purpose of the code.  Are there cases where
> > this optional file is inside an optional directory, leading to
> > ENOTDIR, instead of ENOENT, observed and reported by your check?
> 
> "git grep -B1 warn_on_inaccessible" is enlightening.  I wonder if we
> want to wrap the two lines into a hard to misuse helper function,
> something along this line.  Would having this patch as a preparatory
> step shrink your series?  The patch count would be the same, but you
> wouldn't be writing "if (errno != ENOENT)" lines yourself.

I had a similar thought while reading through it. I think it would be shorter still with:

  FILE *fopen_or_warn(const char *path, const char *mode)
  {
	FILE *fh = fopen(path, mode);
	if (!fh)
		warn_failure_to_read_open_optional_path(path);
	return fh;
  }

And then quite a few of the patches could just be s/fopen/fopen_or_warn/.

-Peff