Re: [PATCH 0/3] avoiding unintended consequences of git_path() usage

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On Wed, Nov 16, 2011 at 01:59:55AM -0600, Jonathan Nieder wrote:
> Ramkumar Ramachandra wrote:
> > Junio C Hamano wrote:
> 
> >> Or perhaps http://thread.gmane.org/gmane.comp.version-control.git/184963/focus=185436
> >
> > I noticed that sha1_to_hex() also operates like this.  The
> > resolve_ref() function is really important, but using the same
> > technique for these tiny functions is probably an overkill
> 
> I don't follow.  Do you mean that not being confusing is overkill,
> because the function is small that no one will bother to look up the
> right semantics?  Wait, that sentence didn't come out the way I
> wanted. ;-)
> 
> Jokes aside, here's a rough series to do the git_path ->
> git_path_unsafe renaming.  While writing it, I noticed a couple of
> bugs, hence the two patches before the last one.  Patch 2 is the more
> interesting one.

Or perhaps we can use per-file buffer rings instead of a global one.
This means git_path() can only interfere another one in the same file,
making the interaction simpler and hopefully simple enough for reviewers
to catch 90% bugs, therefore safe enough to avoid the _unsafe suffix.

Adding static variable declaration in cache.h is ugly, but that could be
moved to a separate header file.

diff --git a/cache.h b/cache.h
index 2e6ad36..437bc3a 100644
--- a/cache.h
+++ b/cache.h
@@ -660,9 +660,13 @@ extern char *git_snpath(char *buf, size_t n, const char *fmt, ...)
 extern char *git_pathdup(const char *fmt, ...)
 	__attribute__((format (printf, 1, 2)));
 
+#define git_path(...) git_path_1(pathname_array[3 & ++pathname_index], __VA_ARGS__)
+static char pathname_array[4][PATH_MAX];
+static int pathname_index;
+
 /* 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)));
+extern char *git_path_1(char *pathname, const char *fmt, ...) __attribute__((format (printf, 2, 3)));
 extern char *git_path_submodule(const char *path, const char *fmt, ...)
 	__attribute__((format (printf, 2, 3)));
 
diff --git a/path.c b/path.c
index b6f71d1..3c95db1 100644
--- a/path.c
+++ b/path.c
@@ -101,10 +101,9 @@ char *mkpath(const char *fmt, ...)
 	return cleanup_path(pathname);
 }
 
-char *git_path(const char *fmt, ...)
+char *git_path_1(char *pathname, const char *fmt, ...)
 {
 	const char *git_dir = get_git_dir();
-	char *pathname = get_pathname();
 	va_list args;
 	unsigned len;