[PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[Alex Riesen]

Both are actually just vsnprintf's but additionally call cleanup_path
on the result. To be used as alternatives to mkpath and git_path where
the buffer for the created path may not be reused by subsequent calls
of the same formatting function.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 cache.h |    4 ++++
 path.c  |   38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index b0edbf9..a9024db 100644
--- a/cache.h
+++ b/cache.h
@@ -495,6 +495,10 @@ extern int check_repository_format(void);
 #define DATA_CHANGED    0x0020
 #define TYPE_CHANGED    0x0040
 
+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)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 76e8872..85ab28a 100644
--- a/path.c
+++ b/path.c
@@ -32,6 +32,44 @@ static char *cleanup_path(char *path)
 	return path;
 }
 
+char *mksnpath(char *buf, size_t n, const char *fmt, ...)
+{
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(buf, n, fmt, args);
+	va_end(args);
+	if (len >= n) {
+		snprintf(buf, n, bad_path);
+		return buf;
+	}
+	return cleanup_path(buf);
+}
+
+char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+{
+	const char *git_dir = get_git_dir();
+	va_list args;
+	size_t len;
+
+	len = strlen(git_dir);
+	if (n < len + 1)
+		goto bad;
+	memcpy(buf, git_dir, len);
+	if (len && !is_dir_sep(git_dir[len-1]))
+		buf[len++] = '/';
+	va_start(args, fmt);
+	len += vsnprintf(buf + len, n - len, fmt, args);
+	va_end(args);
+	if (len >= n)
+		goto bad;
+	return cleanup_path(buf);
+bad:
+	snprintf(buf, n, bad_path);
+	return buf;
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.540.g3f8b

[PATCH] Fix mkpath abuse in dwim_ref and dwim_log of sha1_name.c

[Alex Riesen]

Otherwise the function sometimes fail to resolve obviously correct
refnames, because the string data pointed to by "str" argument were
reused.

The change in dwim_log does not fix anything, just optimizes away
strcpy code as the path can be created directly in the available
buffer.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---

Was noticed in cygwin port, which somehow (supposedly by excessive
calling of git_config from lstat stub setup) managed to reuse the
just returned buffer.

 sha1_name.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 41b6809..159c2ab 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -245,11 +245,13 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 
 	*ref = NULL;
 	for (p = ref_rev_parse_rules; *p; p++) {
+		char fullref[PATH_MAX];
 		unsigned char sha1_from_ref[20];
 		unsigned char *this_result;
 
 		this_result = refs_found ? sha1_from_ref : sha1;
-		r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
+		mksnpath(fullref, sizeof(fullref), *p, len, str);
+		r = resolve_ref(fullref, this_result, 1, NULL);
 		if (r) {
 			if (!refs_found++)
 				*ref = xstrdup(r);
@@ -272,7 +274,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		char path[PATH_MAX];
 		const char *ref, *it;
 
-		strcpy(path, mkpath(*p, len, str));
+		mksnpath(path, sizeof(path), *p, len, str);
 		ref = resolve_ref(path, hash, 1, NULL);
 		if (!ref)
 			continue;
-- 
1.6.0.3.540.g3f8b

[PATCH] Fix potentially dangerous uses of mkpath and git_path

[Alex Riesen]

Replace them  with mksnpath/git_snpath and a local buffer
for the resulting string.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 builtin-apply.c        |    4 ++--
 builtin-for-each-ref.c |    6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index cfd8fce..4c4d1e1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2841,8 +2841,8 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned
 		unsigned int nr = getpid();
 
 		for (;;) {
-			const char *newpath;
-			newpath = mkpath("%s~%u", path, nr);
+			char newpath[PATH_MAX];
+			mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
 			if (!try_create_file(newpath, mode, buf, size)) {
 				if (!rename(newpath, path))
 					return;
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index fa6c1ed..e46b7ad 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -620,14 +620,16 @@ static char *get_short_ref(struct refinfo *ref)
 		for (j = 0; j < i; j++) {
 			const char *rule = ref_rev_parse_rules[j];
 			unsigned char short_objectname[20];
+			char refname[PATH_MAX];
 
 			/*
 			 * the short name is ambiguous, if it resolves
 			 * (with this previous rule) to a valid ref
 			 * read_ref() returns 0 on success
 			 */
-			if (!read_ref(mkpath(rule, short_name_len, short_name),
-				      short_objectname))
+			mksnpath(refname, sizeof(refname),
+				 rule, short_name_len, short_name);
+			if (!read_ref(refname, short_objectname))
 				break;
 		}
 
-- 
1.6.0.3.540.g3f8b

Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[Junio C Hamano]

Where is git_snpath() used?

Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[Alex Riesen]

Junio C Hamano, Mon, Oct 27, 2008 06:07:24 +0100:
> Where is git_snpath() used?

Nowhere yet, but it should replace git_path in every call where the
result is not used immediately. Which, as the story with cygwin
porting shows, can be sometimes not quite trivial (who could suspect
lstat(2) will have application local side effects?).

Maybe I should resend the patches without it, following by patches
introducing git_snpath and replacing calls to git_path.

Maybe Linus should be sued for introducing the function in the first
place.

Re: [PATCH] Fix potentially dangerous uses of mkpath and git_path

[Johannes Sixt]

Alex Riesen schrieb:
> Replace them  with mksnpath/git_snpath and a local buffer
> for the resulting string.

You should describe your definition of "potentially dangerous" to save the
reader some time.

-- Hannes

Re: [PATCH] Fix potentially dangerous uses of mkpath and git_path

[Alex Riesen]

2008/10/27 Johannes Sixt <j.sixt@viscovery.net>:
> Alex Riesen schrieb:
>> Replace them  with mksnpath/git_snpath and a local buffer
>> for the resulting string.
>
> You should describe your definition of "potentially dangerous" to save the
> reader some time.
>

Yeah. Thought it was obvious. Will do.

Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> Junio C Hamano, Mon, Oct 27, 2008 06:07:24 +0100:
>> Where is git_snpath() used?
>
> Nowhere yet, but it should replace git_path in every call where the
> result is not used immediately. Which, as the story with cygwin
> porting shows, can be sometimes not quite trivial (who could suspect
> lstat(2) will have application local side effects?).
>
> Maybe I should resend the patches without it, following by patches
> introducing git_snpath and replacing calls to git_path.

I took the liberty of doing the first half of just that ;-)

I also think this series of fix is maint-worthy, and splitted the last one
into two so that maint and master can be fixed independently.

Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[Alex Riesen]

[-- Attachment #1: Type: text/plain, Size: 545 bytes --]

2008/10/28 Junio C Hamano <gitster@pobox.com>:
> Alex Riesen <raa.lkml@gmail.com> writes:
>> Maybe I should resend the patches without it, following by patches
>> introducing git_snpath and replacing calls to git_path.
>
> I took the liberty of doing the first half of just that ;-)
>

Thanks. And am sorry... I did that too, and stupidly forgot to send.
I also considered replacing xstrdup(mkpath) with a function which does
just that (patches 8-9). Patches 1 and 2 are unrelated, will send them
separately.

FWIW now, I'm sending the patches.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Add-mksnpath-which-allows-to-specify-the-output-buff.patch --]
[-- Type: text/x-diff; name=0003-Add-mksnpath-which-allows-to-specify-the-output-buff.patch, Size: 1653 bytes --]

From dcea4c409f4deadbc176b43e8cdbfd5cd9edac3f Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 10:18:06 +0100
Subject: [PATCH] Add mksnpath which allows to specify the output buffer

To be used as alternative to mkpath where the buffer for the created
path may not be be reused by subsequent calls of the function or will
be copied anyway.

It is actually just a vsnprintf's but additionally calls cleanup_path
on the result.
---
 cache.h |    2 ++
 path.c  |   15 +++++++++++++++
 2 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index b0edbf9..9efa602 100644
--- a/cache.h
+++ b/cache.h
@@ -495,6 +495,8 @@ extern int check_repository_format(void);
 #define DATA_CHANGED    0x0020
 #define TYPE_CHANGED    0x0040
 
+extern char *mksnpath(char *buf, size_t n, const char *fmt, ...)
+	__attribute__((format (printf, 3, 4)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 76e8872..8b64878 100644
--- a/path.c
+++ b/path.c
@@ -32,6 +32,21 @@ static char *cleanup_path(char *path)
 	return path;
 }
 
+char *mksnpath(char *buf, size_t n, const char *fmt, ...)
+{
+	va_list args;
+	unsigned len;
+
+	va_start(args, fmt);
+	len = vsnprintf(buf, n, fmt, args);
+	va_end(args);
+	if (len >= n) {
+		snprintf(buf, n, bad_path);
+		return buf;
+	}
+	return cleanup_path(buf);
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0004-Fix-mkpath-abuse-in-dwim_ref-sha1_name.c.patch --]
[-- Type: text/x-diff; name=0004-Fix-mkpath-abuse-in-dwim_ref-sha1_name.c.patch, Size: 1438 bytes --]

From 14fcf0fe5cd3baec1706a81876698a27adcda1fa Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Tue, 14 Oct 2008 18:23:49 +0200
Subject: [PATCH] Fix mkpath abuse in dwim_ref/sha1_name.c

Otherwise the function sometimes fail to resolve obviously correct refnames,
because the string data pointed to by "ref" argument were reused.

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
 sha1_name.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 41b6809..159c2ab 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -245,11 +245,13 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 
 	*ref = NULL;
 	for (p = ref_rev_parse_rules; *p; p++) {
+		char fullref[PATH_MAX];
 		unsigned char sha1_from_ref[20];
 		unsigned char *this_result;
 
 		this_result = refs_found ? sha1_from_ref : sha1;
-		r = resolve_ref(mkpath(*p, len, str), this_result, 1, NULL);
+		mksnpath(fullref, sizeof(fullref), *p, len, str);
+		r = resolve_ref(fullref, this_result, 1, NULL);
 		if (r) {
 			if (!refs_found++)
 				*ref = xstrdup(r);
@@ -272,7 +274,7 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **log)
 		char path[PATH_MAX];
 		const char *ref, *it;
 
-		strcpy(path, mkpath(*p, len, str));
+		mksnpath(path, sizeof(path), *p, len, str);
 		ref = resolve_ref(path, hash, 1, NULL);
 		if (!ref)
 			continue;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0005-Fix-potentially-dangerous-use-of-mkpath.patch --]
[-- Type: text/x-diff; name=0005-Fix-potentially-dangerous-use-of-mkpath.patch, Size: 2109 bytes --]

From 9f751c2a681aed2089ba30f64a0478ea3e68a81c Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Sun, 19 Oct 2008 20:17:17 +0200
Subject: [PATCH] Fix potentially dangerous use of mkpath

In the changed code a pointer to the buffer returned by mkpath is used
after a function is called which also uses mkpath or git_path.  As
both these functions use the same ring of buffers, the data pointed by
the pointer stored in the first function can be overwritten when the
function returns, not to mention the possibility that other code using
the same buffer ring can come in in the future.

Replace mkpath with mksnpath and a local buffer for the resulting
string.
---
 builtin-apply.c        |    4 ++--
 builtin-for-each-ref.c |    6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index cfd8fce..4c4d1e1 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -2841,8 +2841,8 @@ static void create_one_file(char *path, unsigned mode, const char *buf, unsigned
 		unsigned int nr = getpid();
 
 		for (;;) {
-			const char *newpath;
-			newpath = mkpath("%s~%u", path, nr);
+			char newpath[PATH_MAX];
+			mksnpath(newpath, sizeof(newpath), "%s~%u", path, nr);
 			if (!try_create_file(newpath, mode, buf, size)) {
 				if (!rename(newpath, path))
 					return;
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index fa6c1ed..e46b7ad 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -620,14 +620,16 @@ static char *get_short_ref(struct refinfo *ref)
 		for (j = 0; j < i; j++) {
 			const char *rule = ref_rev_parse_rules[j];
 			unsigned char short_objectname[20];
+			char refname[PATH_MAX];
 
 			/*
 			 * the short name is ambiguous, if it resolves
 			 * (with this previous rule) to a valid ref
 			 * read_ref() returns 0 on success
 			 */
-			if (!read_ref(mkpath(rule, short_name_len, short_name),
-				      short_objectname))
+			mksnpath(refname, sizeof(refname),
+				 rule, short_name_len, short_name);
+			if (!read_ref(refname, short_objectname))
 				break;
 		}
 
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: 0006-Add-git_snpath-a-.git-path-formatting-routine-with.patch --]
[-- Type: text/x-diff; name=0006-Add-git_snpath-a-.git-path-formatting-routine-with.patch, Size: 1881 bytes --]

From cf5fe91d172aca73b79c05aef261fb7040749127 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 10:22:21 +0100
Subject: [PATCH] Add git_snpath: a .git path formatting routine with output buffer

The function's purpose is to replace git_path where the buffer of
formatted path may not be reused by subsequent calls of the function
or will be copied anyway.
---
 cache.h |    2 ++
 path.c  |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index 9efa602..a9024db 100644
--- a/cache.h
+++ b/cache.h
@@ -497,6 +497,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)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 8b64878..85ab28a 100644
--- a/path.c
+++ b/path.c
@@ -47,6 +47,29 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	return cleanup_path(buf);
 }
 
+char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+{
+	const char *git_dir = get_git_dir();
+	va_list args;
+	size_t len;
+
+	len = strlen(git_dir);
+	if (n < len + 1)
+		goto bad;
+	memcpy(buf, git_dir, len);
+	if (len && !is_dir_sep(git_dir[len-1]))
+		buf[len++] = '/';
+	va_start(args, fmt);
+	len += vsnprintf(buf + len, n - len, fmt, args);
+	va_end(args);
+	if (len >= n)
+		goto bad;
+	return cleanup_path(buf);
+bad:
+	snprintf(buf, n, bad_path);
+	return buf;
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0007-Fix-potentially-dangerous-use-of-git_path-in-ref.c.patch --]
[-- Type: text/x-diff; name=0007-Fix-potentially-dangerous-use-of-git_path-in-ref.c.patch, Size: 1506 bytes --]

From 109d1b74e2b093bd63419fb1d0f720fa54a476f3 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 11:11:40 +0100
Subject: [PATCH] Fix potentially dangerous use of git_path in ref.c

---
 refs.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/refs.c b/refs.c
index 0a126fa..68189ba 100644
--- a/refs.c
+++ b/refs.c
@@ -413,7 +413,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		*flag = 0;
 
 	for (;;) {
-		const char *path = git_path("%s", ref);
+		char path[PATH_MAX];
 		struct stat st;
 		char *buf;
 		int fd;
@@ -421,6 +421,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		if (--depth < 0)
 			return NULL;
 
+		git_snpath(path, sizeof(path), "%s", ref);
 		/* Special case: non-existing file. */
 		if (lstat(path, &st) < 0) {
 			struct ref_list *list = get_packed_refs();
@@ -1130,13 +1131,14 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	int logfd, written, oflags = O_APPEND | O_WRONLY;
 	unsigned maxlen, len;
 	int msglen;
-	char *log_file, *logrec;
+	char log_file[PATH_MAX];
+	char *logrec;
 	const char *committer;
 
 	if (log_all_ref_updates < 0)
 		log_all_ref_updates = !is_bare_repository();
 
-	log_file = git_path("logs/%s", ref_name);
+	git_snpath(log_file, sizeof(log_file), "logs/%s", ref_name);
 
 	if (log_all_ref_updates &&
 	    (!prefixcmp(ref_name, "refs/heads/") ||
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #7: 0008-git_pathdup-returns-xstrdup-ed-copy-of-the-formatte.patch --]
[-- Type: text/x-diff; name=0008-git_pathdup-returns-xstrdup-ed-copy-of-the-formatte.patch, Size: 2252 bytes --]

From bb22051c0925039b5161bc0f49e3098f9965a069 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 11:17:51 +0100
Subject: [PATCH] git_pathdup: returns xstrdup-ed copy of the formatted path

---
 cache.h |    2 ++
 path.c  |   24 ++++++++++++++++++++----
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/cache.h b/cache.h
index a9024db..981b4e6 100644
--- a/cache.h
+++ b/cache.h
@@ -499,6 +499,8 @@ 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 char *git_pathdup(const char *fmt, ...)
+	__attribute__((format (printf, 1, 2)));
 /* Return a statically allocated filename matching the sha1 signature */
 extern char *mkpath(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
 extern char *git_path(const char *fmt, ...) __attribute__((format (printf, 1, 2)));
diff --git a/path.c b/path.c
index 85ab28a..092ce57 100644
--- a/path.c
+++ b/path.c
@@ -47,10 +47,9 @@ char *mksnpath(char *buf, size_t n, const char *fmt, ...)
 	return cleanup_path(buf);
 }
 
-char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+static char *git_vsnpath(char *buf, size_t n, const char *fmt, va_list args)
 {
 	const char *git_dir = get_git_dir();
-	va_list args;
 	size_t len;
 
 	len = strlen(git_dir);
@@ -59,9 +58,7 @@ char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 	memcpy(buf, git_dir, len);
 	if (len && !is_dir_sep(git_dir[len-1]))
 		buf[len++] = '/';
-	va_start(args, fmt);
 	len += vsnprintf(buf + len, n - len, fmt, args);
-	va_end(args);
 	if (len >= n)
 		goto bad;
 	return cleanup_path(buf);
@@ -70,6 +67,25 @@ bad:
 	return buf;
 }
 
+char *git_snpath(char *buf, size_t n, const char *fmt, ...)
+{
+	va_list args;
+	va_start(args, fmt);
+	(void)git_vsnpath(buf, n, fmt, args);
+	va_end(args);
+	return buf;
+}
+
+char *git_pathdup(const char *fmt, ...)
+{
+	char path[PATH_MAX];
+	va_list args;
+	va_start(args, fmt);
+	(void)git_vsnpath(path, sizeof(path), fmt, args);
+	va_end(args);
+	return xstrdup(path);
+}
+
 char *mkpath(const char *fmt, ...)
 {
 	va_list args;
-- 
1.6.0.3.549.gb475d


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: 0009-Use-git_pathdup-instead-of-xstrdup-git_path.patch --]
[-- Type: text/x-diff; name=0009-Use-git_pathdup-instead-of-xstrdup-git_path.patch, Size: 5483 bytes --]

From 3e142004e382ea0e05d030a3be6a8fc0e896f5d1 Mon Sep 17 00:00:00 2001
From: Alex Riesen <raa.lkml@gmail.com>
Date: Mon, 27 Oct 2008 11:22:09 +0100
Subject: [PATCH] Use git_pathdup instead of xstrdup(git_path(...))

---
 builtin-config.c |    2 +-
 builtin-reflog.c |    4 ++--
 builtin-revert.c |    2 +-
 builtin-tag.c    |    2 +-
 config.c         |    6 +++---
 environment.c    |    2 +-
 refs.c           |    2 +-
 rerere.c         |    2 +-
 server-info.c    |    2 +-
 9 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin-config.c b/builtin-config.c
index 91fdc49..f710162 100644
--- a/builtin-config.c
+++ b/builtin-config.c
@@ -84,7 +84,7 @@ static int get_value(const char* key_, const char* regex_)
 	local = config_exclusive_filename;
 	if (!local) {
 		const char *home = getenv("HOME");
-		local = repo_config = xstrdup(git_path("config"));
+		local = repo_config = git_pathdup("config");
 		if (git_config_global() && home)
 			global = xstrdup(mkpath("%s/.gitconfig", home));
 		if (git_config_system())
diff --git a/builtin-reflog.c b/builtin-reflog.c
index 6b3667e..d95f515 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -277,11 +277,11 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused,
 	lock = lock_any_ref_for_update(ref, sha1, 0);
 	if (!lock)
 		return error("cannot lock ref '%s'", ref);
-	log_file = xstrdup(git_path("logs/%s", ref));
+	log_file = git_pathdup("logs/%s", ref);
 	if (!file_exists(log_file))
 		goto finish;
 	if (!cmd->dry_run) {
-		newlog_path = xstrdup(git_path("logs/%s.lock", ref));
+		newlog_path = git_pathdup("logs/%s.lock", ref);
 		cb.newlog = fopen(newlog_path, "w");
 	}
 
diff --git a/builtin-revert.c b/builtin-revert.c
index 7483a7a..4038b41 100644
--- a/builtin-revert.c
+++ b/builtin-revert.c
@@ -251,7 +251,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
 	int i, index_fd, clean;
 	char *oneline, *reencoded_message = NULL;
 	const char *message, *encoding;
-	char *defmsg = xstrdup(git_path("MERGE_MSG"));
+	char *defmsg = git_pathdup("MERGE_MSG");
 	struct merge_options o;
 	struct tree *result, *next_tree, *base_tree, *head_tree;
 	static struct lock_file index_lock;
diff --git a/builtin-tag.c b/builtin-tag.c
index b13fa34..efd7723 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -283,7 +283,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 		int fd;
 
 		/* write the template message before editing: */
-		path = xstrdup(git_path("TAG_EDITMSG"));
+		path = git_pathdup("TAG_EDITMSG");
 		fd = open(path, O_CREAT | O_TRUNC | O_WRONLY, 0600);
 		if (fd < 0)
 			die("could not create file '%s': %s",
diff --git a/config.c b/config.c
index b8d289d..67cc1dc 100644
--- a/config.c
+++ b/config.c
@@ -649,7 +649,7 @@ int git_config(config_fn_t fn, void *data)
 		free(user_config);
 	}
 
-	repo_config = xstrdup(git_path("config"));
+	repo_config = git_pathdup("config");
 	ret += git_config_from_file(fn, repo_config, data);
 	free(repo_config);
 	return ret;
@@ -889,7 +889,7 @@ int git_config_set_multivar(const char* key, const char* value,
 	if (config_exclusive_filename)
 		config_filename = xstrdup(config_exclusive_filename);
 	else
-		config_filename = xstrdup(git_path("config"));
+		config_filename = git_pathdup("config");
 
 	/*
 	 * Since "key" actually contains the section name and the real
@@ -1149,7 +1149,7 @@ int git_config_rename_section(const char *old_name, const char *new_name)
 	if (config_exclusive_filename)
 		config_filename = xstrdup(config_exclusive_filename);
 	else
-		config_filename = xstrdup(git_path("config"));
+		config_filename = git_pathdup("config");
 	out_fd = hold_lock_file_for_update(lock, config_filename, 0);
 	if (out_fd < 0) {
 		ret = error("could not lock config file %s", config_filename);
diff --git a/environment.c b/environment.c
index 0693cd9..bf93a59 100644
--- a/environment.c
+++ b/environment.c
@@ -71,7 +71,7 @@ static void setup_git_env(void)
 	}
 	git_graft_file = getenv(GRAFT_ENVIRONMENT);
 	if (!git_graft_file)
-		git_graft_file = xstrdup(git_path("info/grafts"));
+		git_graft_file = git_pathdup("info/grafts");
 }
 
 int is_bare_repository(void)
diff --git a/refs.c b/refs.c
index 68189ba..01e4df4 100644
--- a/refs.c
+++ b/refs.c
@@ -1267,7 +1267,7 @@ int create_symref(const char *ref_target, const char *refs_heads_master,
 	const char *lockpath;
 	char ref[1000];
 	int fd, len, written;
-	char *git_HEAD = xstrdup(git_path("%s", ref_target));
+	char *git_HEAD = git_pathdup("%s", ref_target);
 	unsigned char old_sha1[20], new_sha1[20];
 
 	if (logmsg && read_ref(ref_target, old_sha1))
diff --git a/rerere.c b/rerere.c
index 8e5532b..02931a1 100644
--- a/rerere.c
+++ b/rerere.c
@@ -351,7 +351,7 @@ int setup_rerere(struct string_list *merge_rr)
 	if (!is_rerere_enabled())
 		return -1;
 
-	merge_rr_path = xstrdup(git_path("MERGE_RR"));
+	merge_rr_path = git_pathdup("MERGE_RR");
 	fd = hold_lock_file_for_update(&write_lock, merge_rr_path,
 				       LOCK_DIE_ON_ERROR);
 	read_rr(merge_rr);
diff --git a/server-info.c b/server-info.c
index c1c073b..66b0d9d 100644
--- a/server-info.c
+++ b/server-info.c
@@ -25,7 +25,7 @@ static int add_info_ref(const char *path, const unsigned char *sha1, int flag, v
 
 static int update_info_refs(int force)
 {
-	char *path0 = xstrdup(git_path("info/refs"));
+	char *path0 = git_pathdup("info/refs");
 	int len = strlen(path0);
 	char *path1 = xmalloc(len + 2);
 
-- 
1.6.0.3.549.gb475d

Re: [PATCH] Add mksnpath and git_snpath which allow to specify the output buffer

[Alex Riesen]

Alex Riesen, Tue, Oct 28, 2008 13:47:21 +0100:
> 2008/10/28 Junio C Hamano <gitster@pobox.com>:
> > Alex Riesen <raa.lkml@gmail.com> writes:
> >> Maybe I should resend the patches without it, following by patches
> >> introducing git_snpath and replacing calls to git_path.
> >
> > I took the liberty of doing the first half of just that ;-)
> >
> 
> Thanks. And am sorry... I did that too, and stupidly forgot to send.
> I also considered replacing xstrdup(mkpath) with a function which does
> just that (patches 8-9). Patches 1 and 2 are unrelated, will send them
> separately.
> 
> FWIW now, I'm sending the patches.

Err... The builtin-revert.c hunk of path 0009 depends on the patch I
sent later: "Plug a memleak in builtin-revert". The patch removed const
in front of "char *defmsg".