Re: [PATCH 00/15] Handle fopen() errors
From: Junio C Hamano <hidden>
Date: 2017-04-21 03:41:40
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2017-04-24 · Re: [PATCH 00/15] Handle fopen() errors · Junio C Hamano <hidden>
- 2017-04-21 · Re: [PATCH 00/15] Handle fopen() errors · Junio C Hamano <hidden>
- 2017-04-20 · [PATCH 00/15] Handle fopen() errors · Nguyễn Thái Ngọc Duy <hidden>
Junio C Hamano [off-list ref] 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);