Junio C Hamano <gitster@xxxxxxxxx> 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.
attr.c | 3 +--
git-compat-util.h | 3 +++
wrapper.c | 6 ++++++
3 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 1fcf042b87..f695ded53f 100644
--- a/attr.c
+++ b/attr.c
@@ -373,8 +373,7 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok)
int lineno = 0;
if (!fp) {
- if (errno != ENOENT && errno != ENOTDIR)
- warn_on_inaccessible(path);
+ warn_failure_to_read_open_optional_path(path);
return NULL;
}
res = xcalloc(1, sizeof(*res));
diff --git a/git-compat-util.h b/git-compat-util.h
index 8a4a3f85e7..998366c628 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1094,6 +1094,9 @@ int access_or_die(const char *path, int mode, unsigned flag);
/* Warn on an inaccessible file that ought to be accessible */
void warn_on_inaccessible(const char *path);
+/* Call the above after fopen/open fails for optional input */
+void warn_failure_to_read_open_optional_path(const char *);
+
#ifdef GMTIME_UNRELIABLE_ERRORS
struct tm *git_gmtime(const time_t *);
struct tm *git_gmtime_r(const time_t *, struct tm *);
diff --git a/wrapper.c b/wrapper.c
index 0542fc7582..172cb9fad6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -576,6 +576,12 @@ int remove_or_warn(unsigned int mode, const char *file)
return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file);
}
+void warn_failure_to_read_open_optional_path(const char *path)
+{
+ if (errno != ENOENT && errno != ENOTDIR)
+ warn_on_inaccessible(path);
+}
+
void warn_on_inaccessible(const char *path)
{
warning_errno(_("unable to access '%s'"), path);