All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: git@vger.kernel.org
Cc: j6t@kdbg.org, kusmabite@gmail.com, mlevedahl@gmail.com,
	ramsay@ramsay1.demon.co.uk, tboegi@web.de,
	sunshine@sunshineco.com
Subject: [PATCH 2/2] optimize set_shared_perm()
Date: Mon, 25 Mar 2013 16:57:56 +0100	[thread overview]
Message-ID: <201303251657.57222.tboegi@web.de> (raw)

  optimize set_shared_perm() in path.c:

  - sometimes the chown() function is called even when not needed.
    (This can be provoced by running t1301, and adding some debug code)

    Save a chmod from 400 to 400, or from 600->600 on these files:
    .git/info/refs+
    .git/objects/info/packs+

    Save chmod on directories from 2770 to 2770:
    .git/refs
    .git/refs/heads
    .git/refs/tags

  - as all callers use mode == 0 when caling set_shared_perm(),
    the function can be simplified.
  - all callers use the macro adjust_shared_perm(path) from cache.h
    Convert adjust_shared_perm() from a macro into a function prototype

Signed-off-by: Torsten Bögershausen <tboegi@web.de>
---
 cache.h |  3 +--
 path.c  | 71 +++++++++++++++++++++++++++++++++++------------------------------
 2 files changed, 39 insertions(+), 35 deletions(-)

diff --git a/cache.h b/cache.h
index 59e5b53..65a9db7 100644
--- a/cache.h
+++ b/cache.h
@@ -727,8 +727,7 @@ enum sharedrepo {
 	PERM_EVERYBODY      = 0664
 };
 int git_config_perm(const char *var, const char *value);
-int set_shared_perm(const char *path, int mode);
-#define adjust_shared_perm(path) set_shared_perm((path), 0)
+int adjust_shared_perm(const char *path);
 int safe_create_leading_directories(char *path);
 int safe_create_leading_directories_const(const char *path);
 int mkdir_in_gitdir(const char *path);
diff --git a/path.c b/path.c
index 2fdccc2..4bc918a 100644
--- a/path.c
+++ b/path.c
@@ -1,14 +1,5 @@
 /*
- * I'm tired of doing "vsnprintf()" etc just to open a
- * file, so here's a "return static buffer with printf"
- * interface for paths.
- *
- * It's obviously not thread-safe. Sue me. But it's quite
- * useful for doing things like
- *
- *   f = open(mkpath("%s/%s.git", base, name), O_RDONLY);
- *
- * which is what it's designed for.
+ * Different utilitiy functions for path and path names
  */
 #include "cache.h"
 #include "strbuf.h"
@@ -105,6 +96,13 @@ char *git_pathdup(const char *fmt, ...)
 	return xstrdup(ret);
 }
 
+/*
+ * I'm tired of doing "vsnprintf()" etc just to open a
+ * file, so here's an interface for paths.
+ *
+ * f = open(mkpath("%s/%s.git", base, name), O_RDONLY);
+ *
+ */
 char *mkpathdup(const char *fmt, ...)
 {
 	char *path;
@@ -405,22 +403,13 @@ const char *enter_repo(const char *path, int strict)
 	return NULL;
 }
 
-int set_shared_perm(const char *path, int mode)
+static int calc_shared_perm(int mode)
 {
-	int tweak, shared, orig_mode;
+	int tweak, shared;
 
-	if (!shared_repository) {
-		if (mode)
-			return chmod(path, mode & ~S_IFMT);
-		return 0;
-	}
-	if (!mode) {
-		if (get_st_mode_bits(path, &mode) < 0)
-			return -1;
-		orig_mode = mode;
-	} else
-		orig_mode = 0;
-	if (shared_repository < 0)
+	if (!shared_repository)
+		return mode;
+	else if (shared_repository < 0)
 		shared = -shared_repository;
 	else
 		shared = shared_repository;
@@ -436,16 +425,32 @@ int set_shared_perm(const char *path, int mode)
 	else
 		mode |= tweak;
 
-	if (S_ISDIR(mode)) {
-		/* Copy read bits to execute bits */
-		mode |= (shared & 0444) >> 2;
-		mode |= FORCE_DIR_SET_GID;
-	}
+	return mode;
+}
+
+static int calc_shared_perm_dir(int mode)
+{
+	mode = calc_shared_perm(mode);
+
+	/* Copy read bits to execute bits */
+	mode |= (mode & 0444) >> 2;
+	mode |= FORCE_DIR_SET_GID;
+	return mode;
+}
+
+int adjust_shared_perm(const char *path)
+{
+	int old_mode, new_mode;
+	if (get_st_mode_bits(path, &old_mode) < 0)
+		return -1;
+
+	if (S_ISDIR(old_mode))
+		new_mode = calc_shared_perm_dir(old_mode);
+	else
+		new_mode = calc_shared_perm(old_mode);
 
-	if (((shared_repository < 0
-	      ? (orig_mode & (FORCE_DIR_SET_GID | 0777))
-	      : (orig_mode & mode)) != mode) &&
-	    chmod(path, (mode & ~S_IFMT)) < 0)
+	if (((old_mode ^ new_mode) & ~S_IFMT) &&
+			chmod(path, (new_mode & ~S_IFMT)) < 0)
 		return -2;
 	return 0;
 }
-- 
1.8.2.341.g543621f

             reply	other threads:[~2013-03-25 15:58 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-25 15:57 Torsten Bögershausen [this message]
2013-03-29 21:20 ` [PATCH 2/2] optimize set_shared_perm() Junio C Hamano
2013-03-30  9:53   ` Torsten Bögershausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=201303251657.57222.tboegi@web.de \
    --to=tboegi@web.de \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=kusmabite@gmail.com \
    --cc=mlevedahl@gmail.com \
    --cc=ramsay@ramsay1.demon.co.uk \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.