[PATCH] system_path: use a static buffer

["Carlos Martín Nieto" <cmn@elego.de>]

Make system_path behave like the other path functions by using a
static buffer, fixing a memory leak.

Also make sure the prefix pointer is always initialized to either
PREFIX or NULL.

git_etc_gitattributes and git_etc_gitconfig are the only users who are
affected by this change. Make them use a static buffer, which fits
their use better as well.

Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
---

On mié, 2011-03-16 at 13:43 -0700, Junio C Hamano wrote:
Carlos Martín Nieto <cmn@elego.de> writes:
> 
> > Make system_path behave like the other path functions by using a
> > static buffer, fixing a memory leak.
> >
> > Also make sure the prefix pointer is always initialized to either
> > PREFIX or NULL.
> >
> > Signed-off-by: Carlos Martín Nieto <cmn@elego.de>
> > ---
> 
> Have you made sure all the callers are Ok with this change?
> 
> If somebody called system_path(GIT_EXEC_PATH), saved the result in a
> variable without copying, and then called system_path(ETC_GITATTRIBUTES),
> his variable may now have a value unrelated to GIT_EXEC_PATH, and you
> would fix all such callers to save the value away with strdup().


I checked again, and except for the ones changed in this patch, the
rest copy it to their own buffer or pass it to puts, setenv or
strbuf_addstr.

The way these functions are used suggest the caller expects them to
deal with their own memory, so that's what I've done.

TBH, valgrind only reports a win of ~6-7kB when doing git log on
git.git, but it's a step in the right direction (and adds consistency
to system_path, which is the main win).

 attr.c     |    6 +++---
 config.c   |    6 +++---
 exec_cmd.c |   11 ++++++-----
 3 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/attr.c b/attr.c
index 6aff695..64d803f 100644
--- a/attr.c
+++ b/attr.c
@@ -467,9 +467,9 @@ static void drop_attr_stack(void)
 
 const char *git_etc_gitattributes(void)
 {
-	static const char *system_wide;
-	if (!system_wide)
-		system_wide = system_path(ETC_GITATTRIBUTES);
+	static char system_wide[PATH_MAX];
+	if (!system_wide[0])
+		strlcpy(system_wide, system_path(ETC_GITATTRIBUTES), PATH_MAX);
 	return system_wide;
 }
 
diff --git a/config.c b/config.c
index 822ef83..cd1c295 100644
--- a/config.c
+++ b/config.c
@@ -808,9 +808,9 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)
 
 const char *git_etc_gitconfig(void)
 {
-	static const char *system_wide;
-	if (!system_wide)
-		system_wide = system_path(ETC_GITCONFIG);
+	static char system_wide[PATH_MAX];
+	if (!system_wide[0])
+		strlcpy(system_wide, system_path(ETC_GITCONFIG), PATH_MAX);
 	return system_wide;
 }
 
diff --git a/exec_cmd.c b/exec_cmd.c
index 38545e8..5686952 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -9,11 +9,11 @@ static const char *argv0_path;
 const char *system_path(const char *path)
 {
 #ifdef RUNTIME_PREFIX
-	static const char *prefix;
+	static const char *prefix = NULL;
 #else
 	static const char *prefix = PREFIX;
 #endif
-	struct strbuf d = STRBUF_INIT;
+	static char buf[PATH_MAX];
 
 	if (is_absolute_path(path))
 		return path;
@@ -33,9 +33,10 @@ const char *system_path(const char *path)
 	}
 #endif
 
-	strbuf_addf(&d, "%s/%s", prefix, path);
-	path = strbuf_detach(&d, NULL);
-	return path;
+	if (snprintf(buf, sizeof(buf), "%s/%s", prefix, path) >= sizeof(buf))
+		die("system path too long for %s", path);
+
+	return buf;
 }
 
 const char *git_extract_argv0_path(const char *argv0)
-- 
1.7.4.1