git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 03/34] git_snpath(): retire and replace with strbuf_git_path()

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Nov 30, 2014, 08:24 UTC
Message-ID
<1417335899-27307-4-git-send-email-pclouds@gmail.com>
In-Reply-To
<1417335899-27307-1-git-send-email-pclouds@gmail.com>

In the previous patch, git_snpath() is modified to allocate a new strbuf buffer because vsnpath() needs that. But that makes it awkward because git_snpath() receives a pre-allocated buffer from outside and has to copy data back. Rename it to strbuf_git_path() and make it receive strbuf directly.

Using git_path() in update_refs_for_switch() which used to call git_snpath() is safe because that function and all of its callers do not keep any pointer to the round-robin buffer pool allocated by get_pathname().

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/checkout.c | 13 ++++++++-----
 cache.h            |  4 ++--
 path.c             | 11 ++---------
 refs.c             | 51 ++++++++++++++++++++++++++++++++++++++++++---------
 refs.h             |  2 +-
 5 files changed, 55 insertions(+), 26 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index c600ec1..195aca7 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -588,18 +588,21 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 		if (opts->new_orphan_branch) {
 			if (opts->new_branch_log && !log_all_ref_updates) {
 				int temp;
-				char log_file[PATH_MAX];
-				const char *ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
+				struct strbuf log_file = STRBUF_INIT;
+				int ret;
+				const char *ref_name;
 
+				ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch);
 				temp = log_all_ref_updates;
 				log_all_ref_updates = 1;
-				if (log_ref_setup(ref_name, log_file, sizeof(log_file))) {
+				ret = log_ref_setup(ref_name, &log_file);
+				log_all_ref_updates = temp;
+				strbuf_release(&log_file);
+				if (ret) {
 					fprintf(stderr, _("Can not do reflog for '%s'\n"),
 					    opts->new_orphan_branch);
-					log_all_ref_updates = temp;
 					return;
 				}
-				log_all_ref_updates = temp;
 			}
 		}
 		else
diff --git a/cache.h b/cache.h
index dbee0a3..56643cf 100644
--- a/cache.h
+++ b/cache.h
@@ -679,8 +679,8 @@ extern int check_repository_format(void);
 
 extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
-extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
-	__attribute__((format (printf, 3, 4)));
+extern void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
+	__attribute__((format (printf, 2, 3)));
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
 extern char *mkpathdup(const char *fmt, ...)
diff --git a/path.c b/path.c
index a7ceea2..47753aa 100644
--- a/path.c
+++ b/path.c
@@ -70,19 +70,12 @@ static void vsnpath(struct strbuf *buf, const char *fmt, va_list args)
 	strbuf_cleanup_path(buf);
 }
 
-char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+void strbuf_git_path(struct strbuf *sb, const char *fmt, ...)
 {
-	struct strbuf sb = STRBUF_INIT;
 	va_list args;
 	va_start(args, fmt);
-	vsnpath(&sb, fmt, args);
+	vsnpath(sb, fmt, args);
 	va_end(args);
-	if (sb.len >= n)
-		strlcpy(buf, bad_path, n);
-	else
-		memcpy(buf, sb.buf, sb.len + 1);
-	strbuf_release(&sb);
-	return buf;
 }
 
 char *git_pathdup(const char *fmt, ...)
diff --git a/refs.c b/refs.c
index 23617e0..5ed991b 100644
--- a/refs.c
+++ b/refs.c
@@ -1444,7 +1444,11 @@ static int resolve_missing_loose_ref(const char *refname,
 }
 
 /* This function needs to return a meaningful errno on failure */
-const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned char *sha1, int *flags)
+static const char *resolve_ref_unsafe_1(const char *refname,
+					int resolve_flags,
+					unsigned char *sha1,
+					int *flags,
+					struct strbuf *sb_path)
 {
 	int depth = MAXDEPTH;
 	ssize_t len;
@@ -1475,7 +1479,7 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
 		bad_name = 1;
 	}
 	for (;;) {
-		char path[PATH_MAX];
+		const char *path;
 		struct stat st;
 		char *buf;
 		int fd;
@@ -1485,7 +1489,9 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
 			return NULL;
 		}
 
-		git_snpath(path, sizeof(path), "%s", refname);
+		strbuf_reset(sb_path);
+		strbuf_git_path(sb_path, "%s", refname);
+		path = sb_path->buf;
 
 		/*
 		 * We might have to loop back here to avoid a race
@@ -1612,6 +1618,16 @@ const char *resolve_ref_unsafe(const char *refname, int resolve_flags, unsigned
 	}
 }
 
+const char *resolve_ref_unsafe(const char *refname, int resolve_flags,
+			       unsigned char *sha1, int *flags)
+{
+	struct strbuf sb_path = STRBUF_INIT;
+	const char *ret = resolve_ref_unsafe_1(refname, resolve_flags,
+					       sha1, flags, &sb_path);
+	strbuf_release(&sb_path);
+	return ret;
+}
+
 char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, int *flags)
 {
 	const char *ret = resolve_ref_unsafe(ref, resolve_flags, sha1, flags);
@@ -2941,11 +2957,15 @@ static int copy_msg(char *buf, const char *msg)
 }
 
 /* This function must set a meaningful errno on failure */
-int log_ref_setup(const char *refname, char *logfile, int bufsize)
+int log_ref_setup(const char *refname, struct strbuf *sb_logfile)
 {
 	int logfd, oflags = O_APPEND | O_WRONLY;
+	char *logfile;
 
-	git_snpath(logfile, bufsize, "logs/%s", refname);
+	strbuf_git_path(sb_logfile, "logs/%s", refname);
+	logfile = sb_logfile->buf;
+	/* make sure the rest of the function can't change "logfile" */
+	sb_logfile = NULL;
 	if (log_all_ref_updates &&
 	    (starts_with(refname, "refs/heads/") ||
 	     starts_with(refname, "refs/remotes/") ||
@@ -2990,22 +3010,26 @@ int log_ref_setup(const char *refname, char *logfile, int bufsize)
 	return 0;
 }
 
-static int log_ref_write(const char *refname, const unsigned char *old_sha1,
-			 const unsigned char *new_sha1, const char *msg)
+static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
+			   const unsigned char *new_sha1, const char *msg,
+			   struct strbuf *sb_log_file)
 {
 	int logfd, result, written, oflags = O_APPEND | O_WRONLY;
 	unsigned maxlen, len;
 	int msglen;
-	char log_file[PATH_MAX];
+	const char *log_file;
 	char *logrec;
 	const char *committer;
 
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	result = log_ref_setup(refname, log_file, sizeof(log_file));
+	result = log_ref_setup(refname, sb_log_file);
 	if (result)
 		return result;
+	log_file = sb_log_file->buf;
+	/* make sure the rest of the function can't change "log_file" */
+	sb_log_file = NULL;
 
 	logfd = open(log_file, oflags);
 	if (logfd < 0)
@@ -3038,6 +3062,15 @@ static int log_ref_write(const char *refname, const unsigned char *old_sha1,
 	return 0;
 }
 
+static int log_ref_write(const char *refname, const unsigned char *old_sha1,
+			 const unsigned char *new_sha1, const char *msg)
+{
+	struct strbuf sb = STRBUF_INIT;
+	int ret = log_ref_write_1(refname, old_sha1, new_sha1, msg, &sb);
+	strbuf_release(&sb);
+	return ret;
+}
+
 int is_branch(const char *refname)
 {
 	return !strcmp(refname, "HEAD") || starts_with(refname, "refs/heads/");
diff --git a/refs.h b/refs.h
index 2bc3556..1f88220 100644
--- a/refs.h
+++ b/refs.h
@@ -210,7 +210,7 @@ extern void unlock_ref(struct ref_lock *lock);
 /*
  * Setup reflog before using. Set errno to something meaningful on failure.
  */
-int log_ref_setup(const char *refname, char *logfile, int bufsize);
+int log_ref_setup(const char *refname, struct strbuf *logfile);
 
 /** Reads log for the value of ref during at_time. **/
 extern int read_ref_at(const char *refname, unsigned int flags,

-- 
2.1.0.rc0.78.gc0d8480
Previous: Nguyễn Thái Ngọc DuyNext: Nguyễn Thái Ngọc Duy
Message 4 of 45 in “nd/multiple-work-trees reroll”
  1. 00/34 nd/multiple-work-trees rerollNguyễn Thái Ngọc Duy, Nov 30, 2014
  2. 01/34 path.c: make get_pathname() return strbuf instead of static bufferNguyễn Thái Ngọc Duy, Nov 30, 2014
  3. 02/34 path.c: make get_pathname() call sites return const char *Nguyễn Thái Ngọc Duy, Nov 30, 2014
  4. 03/34 git_snpath(): retire and replace with strbuf_git_path()Nguyễn Thái Ngọc Duy, Nov 30, 2014
  5. 04/34 path.c: rename vsnpath() to do_git_path()Nguyễn Thái Ngọc Duy, Nov 30, 2014
  6. 05/34 path.c: group git_path(), git_pathdup() and strbuf_git_path() togetherNguyễn Thái Ngọc Duy, Nov 30, 2014
  7. 06/34 git_path(): be aware of file relocation in $GIT_DIRNguyễn Thái Ngọc Duy, Nov 30, 2014
  8. 07/34 *.sh: respect $GIT_INDEX_FILENguyễn Thái Ngọc Duy, Nov 30, 2014
  9. 08/34 reflog: avoid constructing .lock path with git_pathNguyễn Thái Ngọc Duy, Nov 30, 2014
  10. 09/34 fast-import: use git_path() for accessing .git dir instead of get_git_dir()Nguyễn Thái Ngọc Duy, Nov 30, 2014
  11. 10/34 commit: use SEQ_DIR instead of hardcoding "sequencer"Nguyễn Thái Ngọc Duy, Nov 30, 2014
  12. 11/34 $GIT_COMMON_DIR: a new environment variableNguyễn Thái Ngọc Duy, Nov 30, 2014
  13. 12/34 git-sh-setup.sh: use rev-parse --git-path to get $GIT_DIR/objectsNguyễn Thái Ngọc Duy, Nov 30, 2014
  14. 13/34 *.sh: avoid hardcoding $GIT_DIR/hooks/...Nguyễn Thái Ngọc Duy, Nov 30, 2014
  15. 14/34 git-stash: avoid hardcoding $GIT_DIR/logs/....Nguyễn Thái Ngọc Duy, Nov 30, 2014
  16. 15/34 setup.c: convert is_git_directory() to use strbufNguyễn Thái Ngọc Duy, Nov 30, 2014
  17. 16/34 setup.c: detect $GIT_COMMON_DIR in is_git_directory()Nguyễn Thái Ngọc Duy, Nov 30, 2014
  18. 17/34 setup.c: convert check_repository_format_gently to use strbufNguyễn Thái Ngọc Duy, Nov 30, 2014
  19. 18/34 setup.c: detect $GIT_COMMON_DIR check_repository_format_gently()Nguyễn Thái Ngọc Duy, Nov 30, 2014
  20. 19/34 setup.c: support multi-checkout repo setupNguyễn Thái Ngọc Duy, Nov 30, 2014
  21. 20/34 wrapper.c: wrapper to open a file, fprintf then closeNguyễn Thái Ngọc Duy, Nov 30, 2014
  22. 21/34 use new wrapper write_file() for simple file writingNguyễn Thái Ngọc Duy, Nov 30, 2014
  23. 22/34 checkout: support checking out into a new working directoryNguyễn Thái Ngọc Duy, Nov 30, 2014
  24. 23/34 prune: strategies for linked checkoutsNguyễn Thái Ngọc Duy, Nov 30, 2014
  25. 24/34 checkout: reject if the branch is already checked out elsewhereNguyễn Thái Ngọc Duy, Nov 30, 2014
  26. Mark LevedahlNov 30, 2014
  27. Duy NguyenDec 1, 2014
  28. Junio C HamanoDec 1, 2014
  29. Mark LevedahlDec 2, 2014
  30. Duy NguyenDec 2, 2014
  31. Junio C HamanoDec 2, 2014
  32. Mark LevedahlDec 3, 2014
  33. Duy NguyenDec 3, 2014
  34. Junio C HamanoDec 3, 2014
  35. Duy NguyenDec 2, 2014
  36. 25/34 checkout: clean up half-prepared directories in --to modeNguyễn Thái Ngọc Duy, Nov 30, 2014
  37. 26/34 gc: style change -- no SP before closing parenthesisNguyễn Thái Ngọc Duy, Nov 30, 2014
  38. 27/34 gc: factor out gc.pruneexpire parsing codeNguyễn Thái Ngọc Duy, Nov 30, 2014
  39. 28/34 gc: support prune --worktreesNguyễn Thái Ngọc Duy, Nov 30, 2014
  40. 29/34 count-objects: report unused files in $GIT_DIR/worktrees/...Nguyễn Thái Ngọc Duy, Nov 30, 2014
  41. 30/34 git_path(): keep "info/sparse-checkout" per work-treeNguyễn Thái Ngọc Duy, Nov 30, 2014
  42. 31/34 checkout: don't require a work tree when checking out into a new oneNguyễn Thái Ngọc Duy, Nov 30, 2014
  43. 32/34 t2025: add a test to make sure grafts is working from a linked checkoutNguyễn Thái Ngọc Duy, Nov 30, 2014
  44. 33/34 checkout: do not fail if target is an empty directoryNguyễn Thái Ngọc Duy, Nov 30, 2014
  45. 34/34 git-common-dir: make "modules/" per-working-directory directoryNguyễn Thái Ngọc Duy, Nov 30, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.