GIT: [PATCH] exec_cmd: system_path memory leak fix

3 messages, 1 author, 2016-06-15 · open the first message on its own page

GIT: [PATCH] exec_cmd: system_path memory leak fix

From: 0xAX <hidden>
Date: 2016-06-15 23:03:05

Hello All,
I found memory leak at exec_cmd.c in system_path function with valgrind.
After this patch valgrind doesn't show any memory leaks, but I'm not sure
that it is the best way to solve this problem. There are a couple of places
that uses system_path, if this way will be good, i'll make another patches
to fix this leak.

Waiting for feedback.

Signed-off-by: Alex Kuleshov <redacted>

[PATCH] exec_cmd: system_path memory leak fix

From: 0xAX <hidden>
Date: 2016-06-15 23:03:05

Signed-off-by: 0xAX <redacted>
---
 exec_cmd.c | 25 ++++++++++++++++---------
 exec_cmd.h |  4 ++--
 git.c      | 12 +++++++++---
 3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 698e752..08f8f80 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -6,7 +6,7 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 #ifdef RUNTIME_PREFIX
 	static const char *prefix;
@@ -14,9 +14,10 @@ const char *system_path(const char *path)
 	static const char *prefix = PREFIX;
 #endif
 	struct strbuf d = STRBUF_INIT;
+	char *new_path = NULL;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 #ifdef RUNTIME_PREFIX
 	assert(argv0_path);
@@ -32,10 +33,13 @@ const char *system_path(const char *path)
 				"Using static fallback '%s'.\n", prefix);
 	}
 #endif
-
 	strbuf_addf(&d, "%s/%s", prefix, path);
-	path = strbuf_detach(&d, NULL);
-	return path;
+	new_path = malloc((strlen(prefix) + strlen(path)) + 2);
+	sprintf(new_path, "%s/%s", prefix, path);
+
+	strbuf_release(&d);
+
+	return new_path;
 }
 
 const char *git_extract_argv0_path(const char *argv0)
@@ -68,16 +72,16 @@ void git_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for git programs. */
-const char *git_exec_path(void)
+char *git_exec_path(void)
 {
 	const char *env;
 
 	if (argv_exec_path)
-		return argv_exec_path;
+		return strdup(argv_exec_path);
 
 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
-		return env;
+		return strdup(env);
 	}
 
 	return system_path(GIT_EXEC_PATH);
@@ -96,7 +100,9 @@ void setup_path(void)
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
 
-	add_path(&new_path, git_exec_path());
+	char *exec_path = git_exec_path();
+
+	add_path(&new_path, exec_path);
 	add_path(&new_path, argv0_path);
 
 	if (old_path)
@@ -107,6 +113,7 @@ void setup_path(void)
 	setenv("PATH", new_path.buf, 1);
 
 	strbuf_release(&new_path);
+	free(exec_path);
 }
 
 const char **prepare_git_cmd(const char **argv)
diff --git a/exec_cmd.h b/exec_cmd.h
index e4c9702..03c8599 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,12 +3,12 @@
 
 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char *git_extract_argv0_path(const char *path);
-extern const char *git_exec_path(void);
+extern char *git_exec_path(void);
 extern void setup_path(void);
 extern const char **prepare_git_cmd(const char **argv);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 LAST_ARG_MUST_BE_NULL
 extern int execl_git_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+extern char *system_path(const char *path);
 
 #endif /* GIT_EXEC_CMD_H */
diff --git a/git.c b/git.c
index 82d7a1c..499dc2a 100644
--- a/git.c
+++ b/git.c
@@ -99,13 +99,19 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 				exit(0);
 			}
 		} else if (!strcmp(cmd, "--html-path")) {
-			puts(system_path(GIT_HTML_PATH));
+			char *git_html_path = system_path(GIT_HTML_PATH);
+			puts(git_html_path);
+			free(git_html_path);
 			exit(0);
 		} else if (!strcmp(cmd, "--man-path")) {
-			puts(system_path(GIT_MAN_PATH));
+			char *git_man_path = system_path(GIT_MAN_PATH);
+			puts(git_man_path);
+			free(git_man_path);
 			exit(0);
 		} else if (!strcmp(cmd, "--info-path")) {
-			puts(system_path(GIT_INFO_PATH));
+			char *git_info_path = system_path(GIT_INFO_PATH);
+			puts(git_info_path);
+			free(git_info_path);
 			exit(0);
 		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
 			use_pager = 1;
-- 
2.2.0.rc3.191.gfdea3f0

Re: [PATCH] exec_cmd: system_path memory leak fix

From: 0xAX <hidden>
Date: 2016-06-15 23:03:05

For example some testing with valgrind:

1. pu (952f0aaadff4afca1497450911587a973c8cca02)

valgrind git help
==27034== Memcheck, a memory error detector
==27034== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==27034== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==27034== Command: git help
==27034==
==27034== HERE GIT HELP OUTPUT
==27034==
==27034== HEAP SUMMARY:
==27034==     in use at exit: 65 bytes in 1 blocks
==27034==   total heap usage: 71 allocs, 70 frees, 11,928 bytes allocated
==27034==
==27034== LEAK SUMMARY:
==27034==    definitely lost: 65 bytes in 1 blocks
==27034==    indirectly lost: 0 bytes in 0 blocks
==27034==      possibly lost: 0 bytes in 0 blocks
==27034==    still reachable: 0 bytes in 0 blocks
==27034==         suppressed: 0 bytes in 0 blocks
==27034== Rerun with --leak-check=full to see details of leaked memory
==27034==
==27034== For counts of detected and suppressed errors, rerun with: -v
==27034== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

2. With patch:

valgrind git help
==28945== Memcheck, a memory error detector
==28945== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==28945== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==28945== Command: git help
==28945==
==28945== HERE git help OUTPUT
==28945==
==28945== HEAP SUMMARY:
==28945==     in use at exit: 0 bytes in 0 blocks
==28945==   total heap usage: 72 allocs, 72 frees, 11,956 bytes allocated
==28945==
==28945== All heap blocks were freed -- no leaks are possible
==28945==
==28945== For counts of detected and suppressed errors, rerun with: -v
==28945== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Of course it is very small leak, but i just started to deep in git
source code. Hope i will be useful for community.

Thank you.

0xAX [off-list ref] @ 2014-11-23 19:56 ALMT:
quoted hunk
Signed-off-by: 0xAX <redacted>
---
 exec_cmd.c | 25 ++++++++++++++++---------
 exec_cmd.h |  4 ++--
 git.c      | 12 +++++++++---
 3 files changed, 27 insertions(+), 14 deletions(-)
diff --git a/exec_cmd.c b/exec_cmd.c
index 698e752..08f8f80 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -6,7 +6,7 @@
 static const char *argv_exec_path;
 static const char *argv0_path;

-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 #ifdef RUNTIME_PREFIX
 	static const char *prefix;
@@ -14,9 +14,10 @@ const char *system_path(const char *path)
 	static const char *prefix = PREFIX;
 #endif
 	struct strbuf d = STRBUF_INIT;
+	char *new_path = NULL;

 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);

 #ifdef RUNTIME_PREFIX
 	assert(argv0_path);
@@ -32,10 +33,13 @@ const char *system_path(const char *path)
 				"Using static fallback '%s'.\n", prefix);
 	}
 #endif
-
 	strbuf_addf(&d, "%s/%s", prefix, path);
-	path = strbuf_detach(&d, NULL);
-	return path;
+	new_path = malloc((strlen(prefix) + strlen(path)) + 2);
+	sprintf(new_path, "%s/%s", prefix, path);
+
+	strbuf_release(&d);
+
+	return new_path;
 }

 const char *git_extract_argv0_path(const char *argv0)
@@ -68,16 +72,16 @@ void git_set_argv_exec_path(const char *exec_path)


 /* Returns the highest-priority, location to look for git programs. */
-const char *git_exec_path(void)
+char *git_exec_path(void)
 {
 	const char *env;

 	if (argv_exec_path)
-		return argv_exec_path;
+		return strdup(argv_exec_path);

 	env = getenv(EXEC_PATH_ENVIRONMENT);
 	if (env && *env) {
-		return env;
+		return strdup(env);
 	}

 	return system_path(GIT_EXEC_PATH);
@@ -96,7 +100,9 @@ void setup_path(void)
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;

-	add_path(&new_path, git_exec_path());
+	char *exec_path = git_exec_path();
+
+	add_path(&new_path, exec_path);
 	add_path(&new_path, argv0_path);

 	if (old_path)
@@ -107,6 +113,7 @@ void setup_path(void)
 	setenv("PATH", new_path.buf, 1);

 	strbuf_release(&new_path);
+	free(exec_path);
 }

 const char **prepare_git_cmd(const char **argv)
diff --git a/exec_cmd.h b/exec_cmd.h
index e4c9702..03c8599 100644
--- a/exec_cmd.h
+++ b/exec_cmd.h
@@ -3,12 +3,12 @@

 extern void git_set_argv_exec_path(const char *exec_path);
 extern const char *git_extract_argv0_path(const char *path);
-extern const char *git_exec_path(void);
+extern char *git_exec_path(void);
 extern void setup_path(void);
 extern const char **prepare_git_cmd(const char **argv);
 extern int execv_git_cmd(const char **argv); /* NULL terminated */
 LAST_ARG_MUST_BE_NULL
 extern int execl_git_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+extern char *system_path(const char *path);

 #endif /* GIT_EXEC_CMD_H */
diff --git a/git.c b/git.c
index 82d7a1c..499dc2a 100644
--- a/git.c
+++ b/git.c
@@ -99,13 +99,19 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 				exit(0);
 			}
 		} else if (!strcmp(cmd, "--html-path")) {
-			puts(system_path(GIT_HTML_PATH));
+			char *git_html_path = system_path(GIT_HTML_PATH);
+			puts(git_html_path);
+			free(git_html_path);
 			exit(0);
 		} else if (!strcmp(cmd, "--man-path")) {
-			puts(system_path(GIT_MAN_PATH));
+			char *git_man_path = system_path(GIT_MAN_PATH);
+			puts(git_man_path);
+			free(git_man_path);
 			exit(0);
 		} else if (!strcmp(cmd, "--info-path")) {
-			puts(system_path(GIT_INFO_PATH));
+			char *git_info_path = system_path(GIT_INFO_PATH);
+			puts(git_info_path);
+			free(git_info_path);
 			exit(0);
 		} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
 			use_pager = 1;
--
Best regards.
0xAX
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help