From ba8bd8300a544959159f6bd3a7e03ac54f85ea3a Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 21 Aug 2012 02:10:59 -0400 Subject: [PATCH 1/4] config: warn on inaccessible files Before reading a config file, we check "!access(path, R_OK)" to make sure that the file exists and is readable. If it's not, then we silently ignore it. For the case of ENOENT, this is fine, as the presence of the file is optional. For other cases, though, it may indicate a configuration error (e.g., not having permissions to read the file). Let's print a warning in these cases to let the user know. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- builtin/config.c | 4 ++-- config.c | 10 +++++----- git-compat-util.h | 3 +++ wrapper.c | 8 ++++++++ 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/builtin/config.c b/builtin/config.c index 8cd08da991..b0394efac9 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -396,8 +396,8 @@ int cmd_config(int argc, const char **argv, const char *prefix) */ die("$HOME not set"); - if (access(user_config, R_OK) && - xdg_config && !access(xdg_config, R_OK)) + if (access_or_warn(user_config, R_OK) && + xdg_config && !access_or_warn(xdg_config, R_OK)) given_config_file = xdg_config; else given_config_file = user_config; diff --git a/config.c b/config.c index 2b706ea205..08e47e2e48 100644 --- a/config.c +++ b/config.c @@ -60,7 +60,7 @@ static int handle_path_include(const char *path, struct config_include_data *inc path = buf.buf; } - if (!access(path, R_OK)) { + if (!access_or_warn(path, R_OK)) { if (++inc->depth > MAX_INCLUDE_DEPTH) die(include_depth_advice, MAX_INCLUDE_DEPTH, path, cf && cf->name ? cf->name : "the command line"); @@ -939,23 +939,23 @@ int git_config_early(config_fn_t fn, void *data, const char *repo_config) home_config_paths(&user_config, &xdg_config, "config"); - if (git_config_system() && !access(git_etc_gitconfig(), R_OK)) { + if (git_config_system() && !access_or_warn(git_etc_gitconfig(), R_OK)) { ret += git_config_from_file(fn, git_etc_gitconfig(), data); found += 1; } - if (xdg_config && !access(xdg_config, R_OK)) { + if (xdg_config && !access_or_warn(xdg_config, R_OK)) { ret += git_config_from_file(fn, xdg_config, data); found += 1; } - if (user_config && !access(user_config, R_OK)) { + if (user_config && !access_or_warn(user_config, R_OK)) { ret += git_config_from_file(fn, user_config, data); found += 1; } - if (repo_config && !access(repo_config, R_OK)) { + if (repo_config && !access_or_warn(repo_config, R_OK)) { ret += git_config_from_file(fn, repo_config, data); found += 1; } diff --git a/git-compat-util.h b/git-compat-util.h index 35b095e8ae..5a520e2b73 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -604,6 +604,9 @@ int rmdir_or_warn(const char *path); */ int remove_or_warn(unsigned int mode, const char *path); +/* Call access(2), but warn for any error besides ENOENT. */ +int access_or_warn(const char *path, int mode); + /* Get the passwd entry for the UID of the current process. */ struct passwd *xgetpwuid_self(void); diff --git a/wrapper.c b/wrapper.c index b5e33e49c7..b40c7e73da 100644 --- a/wrapper.c +++ b/wrapper.c @@ -403,6 +403,14 @@ int remove_or_warn(unsigned int mode, const char *file) return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file); } +int access_or_warn(const char *path, int mode) +{ + int ret = access(path, mode); + if (ret && errno != ENOENT) + warning(_("unable to access '%s': %s"), path, strerror(errno)); + return ret; +} + struct passwd *xgetpwuid_self(void) { struct passwd *pw; From 6966073102170171da4c25a4038bbd143fa6a75b Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 21 Aug 2012 02:26:07 -0400 Subject: [PATCH 2/4] gitignore: report access errors of exclude files When we try to access gitignore files, we check for their existence with a call to "access". We silently ignore missing files. However, if a file is not readable, this may be a configuration error; let's warn the user. For $GIT_DIR/info/excludes or core.excludesfile, we can just use access_or_warn. However, for per-directory files we actually try to open them, so we must add a custom warning. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- dir.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dir.c b/dir.c index 240bf0c49c..ea74048ed3 100644 --- a/dir.c +++ b/dir.c @@ -397,6 +397,8 @@ int add_excludes_from_file_to_list(const char *fname, fd = open(fname, O_RDONLY); if (fd < 0 || fstat(fd, &st) < 0) { + if (errno != ENOENT) + warning(_("unable to access '%s': %s"), fname, strerror(errno)); if (0 <= fd) close(fd); if (!check_index || @@ -1311,9 +1313,9 @@ void setup_standard_excludes(struct dir_struct *dir) home_config_paths(NULL, &xdg_path, "ignore"); excludes_file = xdg_path; } - if (!access(path, R_OK)) + if (!access_or_warn(path, R_OK)) add_excludes_from_file(dir, path); - if (excludes_file && !access(excludes_file, R_OK)) + if (excludes_file && !access_or_warn(excludes_file, R_OK)) add_excludes_from_file(dir, excludes_file); } From 11e50b2736f374f1608e0c5690405be1a74aa16d Mon Sep 17 00:00:00 2001 From: Jeff King Date: Tue, 21 Aug 2012 02:31:52 -0400 Subject: [PATCH 3/4] attr: warn on inaccessible attribute files Just like config and gitignore files, we silently ignore missing or inaccessible attribute files. An existent but inaccessible file is probably a configuration error, so let's warn the user. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- attr.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/attr.c b/attr.c index b52efb55a0..cab01b8b57 100644 --- a/attr.c +++ b/attr.c @@ -352,8 +352,11 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok) char buf[2048]; int lineno = 0; - if (!fp) + if (!fp) { + if (errno != ENOENT) + warning(_("unable to access '%s': %s"), path, strerror(errno)); return NULL; + } res = xcalloc(1, sizeof(*res)); while (fgets(buf, sizeof(buf), fp)) handle_attr_line(res, buf, path, ++lineno, macro_ok); From 55b38a48e2a7ccfaaa7897a5fccb98327fa0e3c0 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 21 Aug 2012 14:52:07 -0700 Subject: [PATCH 4/4] warn_on_inaccessible(): a helper to warn on inaccessible paths The previous series introduced warnings to multiple places, but it could become tiring to see the warning on the same path over and over again during a single run of Git. Making just one function responsible for issuing this warning, we could later choose to keep track of which paths we issued a warning (it would involve a hash table of paths after running them through real_path() or something) in order to reduce noise. Right now we do not know if the noise reduction is necessary, but it still would be a good code reduction/sharing anyway. Signed-off-by: Junio C Hamano --- attr.c | 2 +- dir.c | 2 +- git-compat-util.h | 3 +++ wrapper.c | 7 ++++++- 4 files changed, 11 insertions(+), 3 deletions(-) diff --git a/attr.c b/attr.c index cab01b8b57..f12c83f80a 100644 --- a/attr.c +++ b/attr.c @@ -354,7 +354,7 @@ static struct attr_stack *read_attr_from_file(const char *path, int macro_ok) if (!fp) { if (errno != ENOENT) - warning(_("unable to access '%s': %s"), path, strerror(errno)); + warn_on_inaccessible(path); return NULL; } res = xcalloc(1, sizeof(*res)); diff --git a/dir.c b/dir.c index ea74048ed3..486833986e 100644 --- a/dir.c +++ b/dir.c @@ -398,7 +398,7 @@ int add_excludes_from_file_to_list(const char *fname, fd = open(fname, O_RDONLY); if (fd < 0 || fstat(fd, &st) < 0) { if (errno != ENOENT) - warning(_("unable to access '%s': %s"), fname, strerror(errno)); + warn_on_inaccessible(fname); if (0 <= fd) close(fd); if (!check_index || diff --git a/git-compat-util.h b/git-compat-util.h index 5a520e2b73..000042d793 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -607,6 +607,9 @@ int remove_or_warn(unsigned int mode, const char *path); /* Call access(2), but warn for any error besides ENOENT. */ int access_or_warn(const char *path, int mode); +/* Warn on an inaccessible file that ought to be accessible */ +void warn_on_inaccessible(const char *path); + /* Get the passwd entry for the UID of the current process. */ struct passwd *xgetpwuid_self(void); diff --git a/wrapper.c b/wrapper.c index b40c7e73da..68739aaa3b 100644 --- a/wrapper.c +++ b/wrapper.c @@ -403,11 +403,16 @@ int remove_or_warn(unsigned int mode, const char *file) return S_ISGITLINK(mode) ? rmdir_or_warn(file) : unlink_or_warn(file); } +void warn_on_inaccessible(const char *path) +{ + warning(_("unable to access '%s': %s"), path, strerror(errno)); +} + int access_or_warn(const char *path, int mode) { int ret = access(path, mode); if (ret && errno != ENOENT) - warning(_("unable to access '%s': %s"), path, strerror(errno)); + warn_on_inaccessible(path); return ret; }