git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: git@vger.kernel.org
Cc: Michael Haggerty <mhagger@alum.mit.edu>
Subject: [PATCH 16/17] get_repo_path: refactor path-allocation
Date: Mon, 10 Aug 2015 05:37:55 -0400	[thread overview]
Message-ID: <20150810093754.GP30981@sigill.intra.peff.net> (raw)
In-Reply-To: <20150810092731.GA9027@sigill.intra.peff.net>

The get_repo_path function calls mkpath() and then does some
non-trivial operations on it, like calling
is_git_directory() and read_gitfile(). These are actually
OK (they do not use more pathname static buffers
themselves), but it takes a fair bit of work to verify.

Let's use our own strbuf to store the path, and we can
simply reuse it for each iteration of the loop (we can even
avoid rewriting the beginning part, since we are trying a
series of suffixes).

To make the strbuf cleanup easier, we split out a thin
wrapper. As a bonus, this wrapper can factor out the
canonicalization that happens in all of the early-return
code paths.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/clone.c | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/builtin/clone.c b/builtin/clone.c
index 303a3a7..5864ad1 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -99,51 +99,66 @@ static const char *argv_submodule[] = {
 	"submodule", "update", "--init", "--recursive", NULL
 };
 
-static char *get_repo_path(const char *repo, int *is_bundle)
+static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
 {
 	static char *suffix[] = { "/.git", "", ".git/.git", ".git" };
 	static char *bundle_suffix[] = { ".bundle", "" };
+	size_t baselen = path->len;
 	struct stat st;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(suffix); i++) {
-		const char *path;
-		path = mkpath("%s%s", repo, suffix[i]);
-		if (stat(path, &st))
+		strbuf_setlen(path, baselen);
+		strbuf_addstr(path, suffix[i]);
+		if (stat(path->buf, &st))
 			continue;
-		if (S_ISDIR(st.st_mode) && is_git_directory(path)) {
+		if (S_ISDIR(st.st_mode) && is_git_directory(path->buf)) {
 			*is_bundle = 0;
-			return xstrdup(absolute_path(path));
+			return path->buf;
 		} else if (S_ISREG(st.st_mode) && st.st_size > 8) {
 			/* Is it a "gitfile"? */
 			char signature[8];
-			int len, fd = open(path, O_RDONLY);
+			const char *dst;
+			int len, fd = open(path->buf, O_RDONLY);
 			if (fd < 0)
 				continue;
 			len = read_in_full(fd, signature, 8);
 			close(fd);
 			if (len != 8 || strncmp(signature, "gitdir: ", 8))
 				continue;
-			path = read_gitfile(path);
-			if (path) {
+			dst = read_gitfile(path->buf);
+			if (dst) {
 				*is_bundle = 0;
-				return xstrdup(absolute_path(path));
+				return dst;
 			}
 		}
 	}
 
 	for (i = 0; i < ARRAY_SIZE(bundle_suffix); i++) {
-		const char *path;
-		path = mkpath("%s%s", repo, bundle_suffix[i]);
-		if (!stat(path, &st) && S_ISREG(st.st_mode)) {
+		strbuf_setlen(path, baselen);
+		strbuf_addstr(path, bundle_suffix[i]);
+		if (!stat(path->buf, &st) && S_ISREG(st.st_mode)) {
 			*is_bundle = 1;
-			return xstrdup(absolute_path(path));
+			return path->buf;
 		}
 	}
 
 	return NULL;
 }
 
+static char *get_repo_path(const char *repo, int *is_bundle)
+{
+	struct strbuf path = STRBUF_INIT;
+	const char *raw;
+	char *canon;
+
+	strbuf_addstr(&path, repo);
+	raw = get_repo_path_1(&path, is_bundle);
+	canon = raw ? xstrdup(absolute_path(raw)) : NULL;
+	strbuf_release(&path);
+	return canon;
+}
+
 static char *guess_dir_name(const char *repo, int is_bundle, int is_bare)
 {
 	const char *end = repo + strlen(repo), *start;
-- 
2.5.0.414.g670f2a4

  parent reply	other threads:[~2015-08-10  9:38 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-08-10  9:27 [PATCH 0/17] removing questionable uses of git_path Jeff King
2015-08-10  9:32 ` [PATCH 01/17] cache.h: clarify documentation for git_path, et al Jeff King
2015-08-10  9:32 ` [PATCH 02/17] cache.h: complete set of git_path_submodule helpers Jeff King
2015-08-10  9:32 ` [PATCH 03/17] t5700: modernize style Jeff King
2015-08-10  9:34 ` [PATCH 04/17] add_to_alternates_file: don't add duplicate entries Jeff King
2015-08-11  4:00   ` Michael Haggerty
2015-08-11  9:54     ` Jeff King
2015-08-10  9:35 ` [PATCH 05/17] remove hold_lock_file_for_append Jeff King
2015-08-10 22:36   ` Junio C Hamano
2015-08-11  9:38     ` Jeff King
2015-08-10  9:35 ` [PATCH 06/17] prefer git_pathdup to git_path in some possibly-dangerous cases Jeff King
2015-08-10  9:35 ` [PATCH 07/17] prefer mkpathdup to mkpath in assignments Jeff King
2015-08-10  9:35 ` [PATCH 08/17] remote.c: drop extraneous local variable from migrate_file Jeff King
2015-08-10  9:36 ` [PATCH 09/17] refs.c: remove extra git_path calls from read_loose_refs Jeff King
2015-08-10  9:36 ` [PATCH 10/17] path.c: drop git_path_submodule Jeff King
2015-08-10 22:50   ` Junio C Hamano
2015-08-10 22:57     ` Junio C Hamano
2015-08-10 23:52       ` Junio C Hamano
2015-08-11  9:53       ` Jeff King
2015-08-10  9:36 ` [PATCH 11/17] refs.c: simplify strbufs in reflog setup and writing Jeff King
2015-08-10 10:34   ` Michael Haggerty
2015-08-10 12:26     ` Jeff King
2015-08-10  9:36 ` [PATCH 12/17] refs.c: avoid repeated git_path calls in rename_tmp_log Jeff King
2015-08-10  9:37 ` [PATCH 13/17] refs.c: avoid git_path assignment in lock_ref_sha1_basic Jeff King
2015-08-10  9:37 ` [PATCH 14/17] refs.c: remove_empty_directories can take a strbuf Jeff King
2015-08-10  9:37 ` [PATCH 15/17] find_hook: keep our own static buffer Jeff King
2015-08-10  9:37 ` Jeff King [this message]
2015-08-10  9:38 ` [PATCH 17/17] memoize common git-path "constant" files Jeff King
2015-08-10 12:05   ` Michael Haggerty
2015-08-10 12:30     ` Jeff King
2015-08-10 12:06 ` [PATCH 0/17] removing questionable uses of git_path Michael Haggerty
2015-08-10 17:31 ` Junio C Hamano
2015-08-10 17:47   ` Jeff King
2015-08-15  9:05 ` Duy Nguyen

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=20150810093754.GP30981@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).