threads / patch / 26946

patchsystem_path: use a static buffer

Subject: [PATCH] system_path: use a static buffer

## tl;dr

3 messages between Mar 31, 2011 and Mar 31, 2011. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Carlos Martín Nieto· Mar 31, 2011, 14:36 UTC · lore

Make system_path behave like the other path functions by using a static buffer, plugging 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>
---
 Slightly changed version of the patch in pu with the error strings
changed. Junio, do you want this patch at all. I think in your "what's
cooking" messages, you said you don't like this type of band-aid? As
it's not that big a deal, I'll drop it if you don't want it.
 attr.c     |    6 +++---
 config.c   |    6 +++---
 exec_cmd.c |   15 ++++++++++-----
 3 files changed, 16 insertions(+), 11 deletions(-)
Show changes to 3 files +16 −11

attr.c, config.c, exec_cmd.c

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..8d0fa49 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -9,11 +9,12 @@ 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];
+	int ret;
 
 	if (is_absolute_path(path))
 		return path;
@@ -33,9 +34,13 @@ const char *system_path(const char *path)
 	}
 #endif
 
-	strbuf_addf(&d, "%s/%s", prefix, path);
-	path = strbuf_detach(&d, NULL);
-	return path;
+	ret = snprintf(buf, sizeof(buf), "%s/%s", prefix, path);
+	if (ret >= sizeof(buf))
+		die("system path too long");
+	else if (ret < 0)
+		die_errno("snprintf reported an error");
+
+	return buf;
 }
 
 const char *git_extract_argv0_path(const char *argv0)
-- 
1.7.4.1
Junio C Hamano· Mar 31, 2011, 22:42 UTC · re: Carlos Martín Nieto · lore

Re: [PATCH] system_path: use a static buffer

Carlos Martín Nieto <cmn@elego.de> writes:
>  Slightly changed version of the patch in pu with the error strings
> changed. Junio, do you want this patch at all. I think in your "what's
> cooking" messages, you said you don't like this type of band-aid? As
> it's not that big a deal, I'll drop it if you don't want it.

The "band-aid" refers to the topic listed after your topic, I think. I didn't even have a comment on your topic in the message.

We'll be in pre-release freeze for 1.7.5 soon, and I want to see patches that deal with internal clean-up held for now and resubmit after 1.7.5 is released. This cycle lasted a bit too long but hopefully we can conclude it by the mid April.

Thanks.
Carlos Martín Nieto· Mar 31, 2011, 23:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] system_path: use a static buffer

On Thu, 2011-03-31 at 15:42 -0700, Junio C Hamano wrote:
Show 9 quoted lines
> Carlos Martín Nieto <cmn@elego.de> writes:
> 
> >  Slightly changed version of the patch in pu with the error strings
> > changed. Junio, do you want this patch at all. I think in your "what's
> > cooking" messages, you said you don't like this type of band-aid? As
> > it's not that big a deal, I'll drop it if you don't want it.
> 
> The "band-aid" refers to the topic listed after your topic, I think.  I
> didn't even have a comment on your topic in the message.
 OK, I misunderstood.
Show 5 quoted lines
> 
> We'll be in pre-release freeze for 1.7.5 soon, and I want to see patches
> that deal with internal clean-up held for now and resubmit after 1.7.5 is
> released.  This cycle lasted a bit too long but hopefully we can conclude
> it by the mid April.
 Fair enough. I'll re-send after the release.
   cmn

← back to recent threads