{"thread":{"id":"33039","subject":"[RFC] optimize set_shared_perm()","startedAt":"2013-02-28T17:17:39Z","lastAt":"2013-02-28T17:17:39Z","messageCount":1,"participants":["Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"210428","messageId":"201302281817.40692.tboegi@web.de","threadId":"33039","inReplyTo":null,"subject":"[RFC] optimize set_shared_perm()","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-02-28T17:17:39Z","receivedAt":"2013-02-28T17:17:39Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"optimize set_shared_perm() in path.c:\n\n- sometimes the chown() function is called even when not needed.\n  (This can be provoced by running t1301, and adding some debug code)\n\n  Save a chmod from 400 to 400, or from 600->600 on these files:\n  .git/info/refs+\n  .git/objects/info/packs+\n\n  Save chmod on directories from 2770 to 2770:\n  .git/refs\n  .git/refs/heads\n  .git/refs/tags\n\n- as all callers use mode == 0 when caling set_shared_perm(),\n  the function can be simplified.\n- all callers use the macro adjust_shared_perm(path) from cache.h\n  Convert adjust_shared_perm() from a macro into a function prototype\n\nSigned-off-by: Torsten Bögershausen <tboegi@web.de>\n---\n cache.h |  3 +--\n path.c  | 75 +++++++++++++++++++++++++++++++++++------------------------------\n 2 files changed, 41 insertions(+), 37 deletions(-)\n\nWhile investigating why cygwin 1.7 failes in t1301, and cygwin 1.5 passes,\nI came across a small refactoring.\n\nMy apologizes if the refactoring in line 431 is wrong:\n\tif (((shared_repository < 0\n\t      ? (orig_mode & (FORCE_DIR_SET_GID | 0777))\n\t      : (orig_mode & mode)) != mode) &&\n\t    chmod(path, (mode & ~S_IFMT)) < 0)\n\n\ndiff --git a/cache.h b/cache.h\nindex 2b192d2..9ea9c70 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -700,8 +700,7 @@ enum sharedrepo {\n \tPERM_EVERYBODY      = 0664\n };\n int git_config_perm(const char *var, const char *value);\n-int set_shared_perm(const char *path, int mode);\n-#define adjust_shared_perm(path) set_shared_perm((path), 0)\n+int adjust_shared_perm(const char *path);\n int safe_create_leading_directories(char *path);\n int safe_create_leading_directories_const(const char *path);\n int mkdir_in_gitdir(const char *path);\ndiff --git a/path.c b/path.c\nindex d3d3f8b..8b7922b 100644\n--- a/path.c\n+++ b/path.c\n@@ -1,14 +1,5 @@\n /*\n- * I'm tired of doing \"vsnprintf()\" etc just to open a\n- * file, so here's a \"return static buffer with printf\"\n- * interface for paths.\n- *\n- * It's obviously not thread-safe. Sue me. But it's quite\n- * useful for doing things like\n- *\n- *   f = open(mkpath(\"%s/%s.git\", base, name), O_RDONLY);\n- *\n- * which is what it's designed for.\n+ * Different utilitiy functions for path and path names\n  */\n #include \"cache.h\"\n #include \"strbuf.h\"\n@@ -89,6 +80,13 @@ char *git_pathdup(const char *fmt, ...)\n \treturn xstrdup(ret);\n }\n \n+/*\n+ * I'm tired of doing \"vsnprintf()\" etc just to open a\n+ * file, so here's an interface for paths.\n+ *\n+ * f = open(mkpath(\"%s/%s.git\", base, name), O_RDONLY);\n+ *\n+ */\n char *mkpathdup(const char *fmt, ...)\n {\n \tchar *path;\n@@ -389,24 +387,13 @@ const char *enter_repo(const char *path, int strict)\n \treturn NULL;\n }\n \n-int set_shared_perm(const char *path, int mode)\n+static int calc_shared_perm(int mode)\n {\n-\tstruct stat st;\n-\tint tweak, shared, orig_mode;\n+\tint tweak, shared;\n \n-\tif (!shared_repository) {\n-\t\tif (mode)\n-\t\t\treturn chmod(path, mode & ~S_IFMT);\n-\t\treturn 0;\n-\t}\n-\tif (!mode) {\n-\t\tif (lstat(path, &st) < 0)\n-\t\t\treturn -1;\n-\t\tmode = st.st_mode;\n-\t\torig_mode = mode;\n-\t} else\n-\t\torig_mode = 0;\n-\tif (shared_repository < 0)\n+\tif (!shared_repository)\n+\t\treturn mode;\n+\telse if (shared_repository < 0)\n \t\tshared = -shared_repository;\n \telse\n \t\tshared = shared_repository;\n@@ -422,16 +409,34 @@ int set_shared_perm(const char *path, int mode)\n \telse\n \t\tmode |= tweak;\n \n-\tif (S_ISDIR(mode)) {\n-\t\t/* Copy read bits to execute bits */\n-\t\tmode |= (shared & 0444) >> 2;\n-\t\tmode |= FORCE_DIR_SET_GID;\n-\t}\n+\treturn mode;\n+}\n+\n+static int calc_shared_perm_dir(int mode)\n+{\n+\tmode = calc_shared_perm(mode);\n+\n+\t/* Copy read bits to execute bits */\n+\tmode |= (mode & 0444) >> 2;\n+\tmode |= FORCE_DIR_SET_GID;\n+\treturn mode;\n+}\n+\n+int adjust_shared_perm(const char *path)\n+{\n+\tstruct stat st;\n+\tint old_mode, new_mode;\n+\tif (lstat(path, &st) < 0)\n+\t\treturn -1;\n+\told_mode = st.st_mode;\n+\n+\tif (S_ISDIR(old_mode))\n+\t\tnew_mode = calc_shared_perm_dir(old_mode);\n+\telse\n+\t\tnew_mode = calc_shared_perm(old_mode);\n \n-\tif (((shared_repository < 0\n-\t      ? (orig_mode & (FORCE_DIR_SET_GID | 0777))\n-\t      : (orig_mode & mode)) != mode) &&\n-\t    chmod(path, (mode & ~S_IFMT)) < 0)\n+\tif (((old_mode ^ new_mode) & ~S_IFMT) &&\n+\t\t\tchmod(path, (new_mode & ~S_IFMT)) < 0)\n \t\treturn -2;\n \treturn 0;\n }\n-- \n1.8.1.1\n"}]}