From 5a688fe4706462dfa0a7932ef0c82c335c47e9ab Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Wed, 25 Mar 2009 16:19:36 -0700 Subject: "core.sharedrepository = 0mode" should set, not loosen This fixes the behaviour of octal notation to how it is defined in the documentation, while keeping the traditional "loosen only" semantics intact for "group" and "everybody". Three main points of this patch are: - For an explicit octal notation, the internal shared_repository variable is set to a negative value, so that we can tell "group" (which is to "OR" in 0660) and 0660 (which is to "SET" to 0660); - git-init did not set shared_repository variable early enough to affect the initial creation of many files, notably copied templates and the configuration. We set it very early when a command-line option specifies a custom value. - Many codepaths create files inside $GIT_DIR by various ways that all involve mkstemp(), and then call move_temp_to_file() to rename it to its final destination. We can add adjust_shared_perm() call here; for the traditional "loosen-only", this would be a no-op for many codepaths because the mode is already loose enough, but with the new behaviour it makes a difference. Signed-off-by: Junio C Hamano --- path.c | 36 +++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 15 deletions(-) (limited to 'path.c') diff --git a/path.c b/path.c index e332b504a6..42898e0fb1 100644 --- a/path.c +++ b/path.c @@ -314,33 +314,39 @@ char *enter_repo(char *path, int strict) int adjust_shared_perm(const char *path) { struct stat st; - int mode; + int mode, tweak, shared; if (!shared_repository) return 0; if (lstat(path, &st) < 0) return -1; mode = st.st_mode; - - if (shared_repository) { - int tweak = shared_repository; - if (!(mode & S_IWUSR)) - tweak &= ~0222; + if (shared_repository < 0) + shared = -shared_repository; + else + shared = shared_repository; + tweak = shared; + + if (!(mode & S_IWUSR)) + tweak &= ~0222; + if (mode & S_IXUSR) + /* Copy read bits to execute bits */ + tweak |= (tweak & 0444) >> 2; + if (shared_repository < 0) + mode = (mode & ~0777) | tweak; + else mode |= tweak; - } else { - /* Preserve old PERM_UMASK behaviour */ - if (mode & S_IWUSR) - mode |= S_IWGRP; - } if (S_ISDIR(mode)) { - mode |= FORCE_DIR_SET_GID; - /* Copy read bits to execute bits */ - mode |= (shared_repository & 0444) >> 2; + mode |= (shared & 0444) >> 2; + mode |= FORCE_DIR_SET_GID; } - if ((mode & st.st_mode) != mode && chmod(path, mode) < 0) + if (((shared_repository < 0 + ? (st.st_mode & (FORCE_DIR_SET_GID | 0777)) + : (st.st_mode & mode)) != mode) && + chmod(path, mode) < 0) return -2; return 0; } -- cgit v1.3 From 17e61b82887fb71800b0fcd39ffe89ddf4d2492e Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Fri, 27 Mar 2009 23:21:00 -0700 Subject: set_shared_perm(): sometimes we know what the final mode bits should look like adjust_shared_perm() first obtains the mode bits from lstat(2), expecting to find what the result of applying user's umask is, and then tweaks it as necessary. When the file to be adjusted is created with mkstemp(3), however, the mode thusly obtained does not have anything to do with user's umask, and we would need to start from 0444 in such a case and there is no point running lstat(2) for such a path. This introduces a new API set_shared_perm() to bypass the lstat(2) and instead force setting the mode bits to the desired value directly. adjust_shared_perm() becomes a thin wrapper to the function. Signed-off-by: Junio C Hamano --- cache.h | 3 ++- path.c | 25 ++++++++++++++++--------- sha1_file.c | 2 +- 3 files changed, 19 insertions(+), 11 deletions(-) (limited to 'path.c') diff --git a/cache.h b/cache.h index 189151de25..e283bbe173 100644 --- a/cache.h +++ b/cache.h @@ -613,7 +613,8 @@ enum sharedrepo { PERM_EVERYBODY = 0664, }; int git_config_perm(const char *var, const char *value); -int adjust_shared_perm(const char *path); +int set_shared_perm(const char *path, int mode); +#define adjust_shared_perm(path) set_shared_perm((path), 0) int safe_create_leading_directories(char *path); int safe_create_leading_directories_const(const char *path); char *enter_repo(char *path, int strict); diff --git a/path.c b/path.c index 42898e0fb1..8a0a6741fd 100644 --- a/path.c +++ b/path.c @@ -311,16 +311,23 @@ char *enter_repo(char *path, int strict) return NULL; } -int adjust_shared_perm(const char *path) +int set_shared_perm(const char *path, int mode) { struct stat st; - int mode, tweak, shared; + int tweak, shared, orig_mode; - if (!shared_repository) + if (!shared_repository) { + if (mode) + return chmod(path, mode & ~S_IFMT); return 0; - if (lstat(path, &st) < 0) - return -1; - mode = st.st_mode; + } + if (!mode) { + if (lstat(path, &st) < 0) + return -1; + mode = st.st_mode; + orig_mode = mode; + } else + orig_mode = 0; if (shared_repository < 0) shared = -shared_repository; else @@ -344,9 +351,9 @@ int adjust_shared_perm(const char *path) } if (((shared_repository < 0 - ? (st.st_mode & (FORCE_DIR_SET_GID | 0777)) - : (st.st_mode & mode)) != mode) && - chmod(path, mode) < 0) + ? (orig_mode & (FORCE_DIR_SET_GID | 0777)) + : (orig_mode & mode)) != mode) && + chmod(path, (mode & ~S_IFMT)) < 0) return -2; return 0; } diff --git a/sha1_file.c b/sha1_file.c index 6f278593e5..d978abf43d 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2280,7 +2280,7 @@ int move_temp_to_file(const char *tmpfile, const char *filename) } out: - if (chmod(filename, 0444) || adjust_shared_perm(filename)) + if (set_shared_perm(filename, (S_IFREG|0444))) return error("unable to set permission to '%s'", filename); return 0; } -- cgit v1.3