On Fri, Apr 21, 2017 at 1:29 PM, Jeff King [off-list ref] wrote:

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

quoted

Junio C Hamano [off-list ref] writes:

quoted

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

Jeff.. oh Jeff.. you have made it _way_ too convenient that after a
quick grep at fopen( again, I found a couple more places that I would
have just ignored last time (too much work), but now all I need to do
is Alt-f to the end of fopen and Alt-/ a few times. Too tempting.. :)
-- 
Duy