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

Subsystems: the rest

DORMANTno replies

5 messages, 2 authors, 2016-06-15 · open the first message on its own page

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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:45:32

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 <redacted>
---
 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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:45:32

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 <redacted>
---

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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:45:32

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

Signed-off-by: Alex Riesen <redacted>
---
 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] Fix potentially dangerous uses of mkpath and git_path

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:45:32

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

From: Alex Riesen <hidden>
Date: 2016-06-15 22:45:32

2008/10/27 Johannes Sixt [off-list ref]:

Alex Riesen schrieb:

quoted

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.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help