linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string
  2015-11-18  6:40 [PATCH perf/core 00/13] perf memory/refcnt leak fixes Masami Hiramatsu
@ 2015-11-18  6:40 ` Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: Masami Hiramatsu @ 2015-11-18  6:40 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, Adrian Hunter, Ingo Molnar,
	Namhyung Kim, Jiri Olsa

Since system_path() returns malloc'd string if given path is
not an absolute path, perf_exec_path sometimes returns static
string and sometimes returns malloc'd string depends on the
environment variables or command options.

This causes a memory leak because caller can not free the
returned string.

This fixes perf_exec_path and system_path to always return
malloc'd string, so caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/exec_cmd.c |   20 ++++++++++++--------
 tools/perf/util/exec_cmd.h |    5 +++--
 tools/perf/util/help.c     |    6 ++++--
 3 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..7031ffc 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,16 +52,18 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	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;
+		env = strdup(env);
+		if (env)
+			return env;
 	}
 
 	return system_path(PERF_EXEC_PATH);
@@ -83,9 +85,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string
@ 2015-11-18 22:23 Arnaldo Carvalho de Melo
  2015-11-19  3:46 ` 平松雅巳 / HIRAMATU,MASAMI
  2015-11-19  6:04 ` [PATCH perf/core v2] " Masami Hiramatsu
  0 siblings, 2 replies; 5+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-11-18 22:23 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Peter Zijlstra, linux-kernel, Adrian Hunter, Ingo Molnar,
	Namhyung Kim, Jiri Olsa

Em Wed, Nov 18, 2015 at 03:40:14PM +0900, Masami Hiramatsu escreveu:
> Since system_path() returns malloc'd string if given path is
> not an absolute path, perf_exec_path sometimes returns static
> string and sometimes returns malloc'd string depends on the
> environment variables or command options.
> 
> This causes a memory leak because caller can not free the
> returned string.
> 
> This fixes perf_exec_path and system_path to always return
> malloc'd string, so caller can always free it.
 
>  /* Returns the highest-priority, location to look for perf programs. */
> -const char *perf_exec_path(void)
> +char *perf_exec_path(void)
>  {
> -	const char *env;
> +	char *env;
>  
>  	if (argv_exec_path)
> -		return argv_exec_path;
> +		return strdup(argv_exec_path);

So here you return strdup without checking if it fails

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

But here you change the behaviour by, having a EXEC_PATH_ENVIRONMENT,
fallback to PERF_EXEC_PATH if strdup fails, that in turn can end up
returning NULL, since system_path() doesn't check the strdup() result?

I wonder if in those cases we couldn't check the address range for the
pointer being freed and realize it is in .bss, .data or that it is a
stack variable? Maybe there is some malloc() friend that, given a
pointer, tells that yeah, it was allocated?

- Arnaldo

>  	}
>  
>  	return system_path(PERF_EXEC_PATH);
> @@ -83,9 +85,11 @@ void setup_path(void)
>  {
>  	const char *old_path = getenv("PATH");
>  	struct strbuf new_path = STRBUF_INIT;
> +	char *tmp = perf_exec_path();
>  
> -	add_path(&new_path, perf_exec_path());
> +	add_path(&new_path, tmp);
>  	add_path(&new_path, argv0_path);
> +	free(tmp);
>  
>  	if (old_path)
>  		strbuf_addstr(&new_path, old_path);
> diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
> index bc4b915..48b4175 100644
> --- a/tools/perf/util/exec_cmd.h
> +++ b/tools/perf/util/exec_cmd.h
> @@ -3,10 +3,11 @@
>  
>  extern void perf_set_argv_exec_path(const char *exec_path);
>  extern const char *perf_extract_argv0_path(const char *path);
> -extern const char *perf_exec_path(void);
>  extern void setup_path(void);
>  extern int execv_perf_cmd(const char **argv); /* NULL terminated */
>  extern int execl_perf_cmd(const char *cmd, ...);
> -extern const char *system_path(const char *path);
> +/* perf_exec_path and system_path return malloc'd string, caller must free it */
> +extern char *perf_exec_path(void);
> +extern char *system_path(const char *path);
>  
>  #endif /* __PERF_EXEC_CMD_H */
> diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
> index 86c37c4..fa1fc4a 100644
> --- a/tools/perf/util/help.c
> +++ b/tools/perf/util/help.c
> @@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
>  		struct cmdnames *other_cmds)
>  {
>  	const char *env_path = getenv("PATH");
> -	const char *exec_path = perf_exec_path();
> +	char *exec_path = perf_exec_path();
>  
>  	if (exec_path) {
>  		list_commands_in_dir(main_cmds, exec_path, prefix);
> @@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
>  		      sizeof(*other_cmds->names), cmdname_compare);
>  		uniq(other_cmds);
>  	}
> +	free(exec_path);
>  	exclude_cmds(other_cmds, main_cmds);
>  }
>  
> @@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
>  			longest = other_cmds->names[i]->len;
>  
>  	if (main_cmds->cnt) {
> -		const char *exec_path = perf_exec_path();
> +		char *exec_path = perf_exec_path();
>  		printf("available %s in '%s'\n", title, exec_path);
>  		printf("----------------");
>  		mput_char('-', strlen(title) + strlen(exec_path));
>  		putchar('\n');
>  		pretty_print_string_list(main_cmds, longest);
>  		putchar('\n');
> +		free(exec_path);
>  	}
>  
>  	if (other_cmds->cnt) {

----- End forwarded message -----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string
  2015-11-18 22:23 [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string Arnaldo Carvalho de Melo
@ 2015-11-19  3:46 ` 平松雅巳 / HIRAMATU,MASAMI
  2015-11-19  6:04 ` [PATCH perf/core v2] " Masami Hiramatsu
  1 sibling, 0 replies; 5+ messages in thread
From: 平松雅巳 / HIRAMATU,MASAMI @ 2015-11-19  3:46 UTC (permalink / raw)
  To: 'Arnaldo Carvalho de Melo'
  Cc: Peter Zijlstra, linux-kernel@vger.kernel.org, Adrian Hunter,
	Ingo Molnar, Namhyung Kim, Jiri Olsa

From: Arnaldo Carvalho de Melo [mailto:acme@kernel.org]
>
>Em Wed, Nov 18, 2015 at 03:40:14PM +0900, Masami Hiramatsu escreveu:
>> Since system_path() returns malloc'd string if given path is
>> not an absolute path, perf_exec_path sometimes returns static
>> string and sometimes returns malloc'd string depends on the
>> environment variables or command options.
>>
>> This causes a memory leak because caller can not free the
>> returned string.
>>
>> This fixes perf_exec_path and system_path to always return
>> malloc'd string, so caller can always free it.
>
>>  /* Returns the highest-priority, location to look for perf programs. */
>> -const char *perf_exec_path(void)
>> +char *perf_exec_path(void)
>>  {
>> -	const char *env;
>> +	char *env;
>>
>>  	if (argv_exec_path)
>> -		return argv_exec_path;
>> +		return strdup(argv_exec_path);
>
>So here you return strdup without checking if it fails
>
>>
>>  	env = getenv(EXEC_PATH_ENVIRONMENT);
>>  	if (env && *env) {
>> -		return env;
>> +		env = strdup(env);
>> +		if (env)
>> +			return env;
>
>But here you change the behaviour by, having a EXEC_PATH_ENVIRONMENT,
>fallback to PERF_EXEC_PATH if strdup fails, that in turn can end up
>returning NULL, since system_path() doesn't check the strdup() result?

Ah, right. actually this is the first part where I fixed, and at that
point I thought this is better. But afterwards, I changed my mind to
return strdup directly. (If there is no memory caller must handle it)
So, I think the above should be

	if (env && *env)
		return strdup(env);

>
>I wonder if in those cases we couldn't check the address range for the
>pointer being freed and realize it is in .bss, .data or that it is a
>stack variable? Maybe there is some malloc() friend that, given a
>pointer, tells that yeah, it was allocated?

I wonder too. But as far as I took a look, I couldn't find such functions.

Thank you,

>
>- Arnaldo
>
>>  	}
>>
>>  	return system_path(PERF_EXEC_PATH);
>> @@ -83,9 +85,11 @@ void setup_path(void)
>>  {
>>  	const char *old_path = getenv("PATH");
>>  	struct strbuf new_path = STRBUF_INIT;
>> +	char *tmp = perf_exec_path();
>>
>> -	add_path(&new_path, perf_exec_path());
>> +	add_path(&new_path, tmp);
>>  	add_path(&new_path, argv0_path);
>> +	free(tmp);
>>
>>  	if (old_path)
>>  		strbuf_addstr(&new_path, old_path);
>> diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
>> index bc4b915..48b4175 100644
>> --- a/tools/perf/util/exec_cmd.h
>> +++ b/tools/perf/util/exec_cmd.h
>> @@ -3,10 +3,11 @@
>>
>>  extern void perf_set_argv_exec_path(const char *exec_path);
>>  extern const char *perf_extract_argv0_path(const char *path);
>> -extern const char *perf_exec_path(void);
>>  extern void setup_path(void);
>>  extern int execv_perf_cmd(const char **argv); /* NULL terminated */
>>  extern int execl_perf_cmd(const char *cmd, ...);
>> -extern const char *system_path(const char *path);
>> +/* perf_exec_path and system_path return malloc'd string, caller must free it */
>> +extern char *perf_exec_path(void);
>> +extern char *system_path(const char *path);
>>
>>  #endif /* __PERF_EXEC_CMD_H */
>> diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
>> index 86c37c4..fa1fc4a 100644
>> --- a/tools/perf/util/help.c
>> +++ b/tools/perf/util/help.c
>> @@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
>>  		struct cmdnames *other_cmds)
>>  {
>>  	const char *env_path = getenv("PATH");
>> -	const char *exec_path = perf_exec_path();
>> +	char *exec_path = perf_exec_path();
>>
>>  	if (exec_path) {
>>  		list_commands_in_dir(main_cmds, exec_path, prefix);
>> @@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
>>  		      sizeof(*other_cmds->names), cmdname_compare);
>>  		uniq(other_cmds);
>>  	}
>> +	free(exec_path);
>>  	exclude_cmds(other_cmds, main_cmds);
>>  }
>>
>> @@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
>>  			longest = other_cmds->names[i]->len;
>>
>>  	if (main_cmds->cnt) {
>> -		const char *exec_path = perf_exec_path();
>> +		char *exec_path = perf_exec_path();
>>  		printf("available %s in '%s'\n", title, exec_path);
>>  		printf("----------------");
>>  		mput_char('-', strlen(title) + strlen(exec_path));
>>  		putchar('\n');
>>  		pretty_print_string_list(main_cmds, longest);
>>  		putchar('\n');
>> +		free(exec_path);
>>  	}
>>
>>  	if (other_cmds->cnt) {
>
>----- End forwarded message -----

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH perf/core v2] perf: Make perf_exec_path always returns malloc'd string
  2015-11-18 22:23 [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string Arnaldo Carvalho de Melo
  2015-11-19  3:46 ` 平松雅巳 / HIRAMATU,MASAMI
@ 2015-11-19  6:04 ` Masami Hiramatsu
  2015-11-23 16:11   ` [tip:perf/core] perf tools: Make perf_exec_path() always return " tip-bot for Masami Hiramatsu
  1 sibling, 1 reply; 5+ messages in thread
From: Masami Hiramatsu @ 2015-11-19  6:04 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, linux-kernel, Adrian Hunter, Ingo Molnar,
	Namhyung Kim, Jiri Olsa

Since system_path() returns malloc'd string if given path is
not an absolute path, perf_exec_path sometimes returns static
string and sometimes returns malloc'd string depends on the
environment variables or command options.

This causes a memory leak because caller can not free the
returned string.

This fixes perf_exec_path and system_path to always return
malloc'd string, so caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
  Changes in v2:
   - do not change the behavior if strdup is failed.
---
 tools/perf/util/exec_cmd.c |   21 +++++++++++----------
 tools/perf/util/exec_cmd.h |    5 +++--
 tools/perf/util/help.c     |    6 ++++--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..1099e92 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,17 +52,16 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	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;
-	}
+	if (env && *env)
+		return strdup(env);
 
 	return system_path(PERF_EXEC_PATH);
 }
@@ -83,9 +82,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [tip:perf/core] perf tools: Make perf_exec_path() always return malloc'd string
  2015-11-19  6:04 ` [PATCH perf/core v2] " Masami Hiramatsu
@ 2015-11-23 16:11   ` tip-bot for Masami Hiramatsu
  0 siblings, 0 replies; 5+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2015-11-23 16:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: mingo, namhyung, hpa, adrian.hunter, linux-kernel,
	masami.hiramatsu.pt, tglx, acme, a.p.zijlstra, jolsa

Commit-ID:  c4068f51d40df151a661a384ab1309b11d7f012e
Gitweb:     http://git.kernel.org/tip/c4068f51d40df151a661a384ab1309b11d7f012e
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Thu, 19 Nov 2015 15:04:53 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 19 Nov 2015 13:19:19 -0300

perf tools: Make perf_exec_path() always return malloc'd string

Since system_path() returns malloc'd string if given path is not an
absolute path, perf_exec_path() sometimes returns a static string and
sometimes returns a malloc'd string depending on the environment
variables or command options.

This may cause a memory leak because the caller can not unconditionally
free the returned string.

This fixes perf_exec_path() and system_path() to always return a
malloc'd string, so the caller can always free it.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20151119060453.14210.65666.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/exec_cmd.c | 21 +++++++++++----------
 tools/perf/util/exec_cmd.h |  5 +++--
 tools/perf/util/help.c     |  6 ++++--
 3 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..1099e92 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,17 @@
 static const char *argv_exec_path;
 static const char *argv0_path;
 
-const char *system_path(const char *path)
+char *system_path(const char *path)
 {
 	static const char *prefix = PREFIX;
 	struct strbuf d = STRBUF_INIT;
 
 	if (is_absolute_path(path))
-		return path;
+		return strdup(path);
 
 	strbuf_addf(&d, "%s/%s", prefix, path);
 	path = strbuf_detach(&d, NULL);
-	return path;
+	return (char *)path;
 }
 
 const char *perf_extract_argv0_path(const char *argv0)
@@ -52,17 +52,16 @@ void perf_set_argv_exec_path(const char *exec_path)
 
 
 /* Returns the highest-priority, location to look for perf programs. */
-const char *perf_exec_path(void)
+char *perf_exec_path(void)
 {
-	const char *env;
+	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;
-	}
+	if (env && *env)
+		return strdup(env);
 
 	return system_path(PERF_EXEC_PATH);
 }
@@ -83,9 +82,11 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
+	char *tmp = perf_exec_path();
 
-	add_path(&new_path, perf_exec_path());
+	add_path(&new_path, tmp);
 	add_path(&new_path, argv0_path);
+	free(tmp);
 
 	if (old_path)
 		strbuf_addstr(&new_path, old_path);
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..48b4175 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,11 @@
 
 extern void perf_set_argv_exec_path(const char *exec_path);
 extern const char *perf_extract_argv0_path(const char *path);
-extern const char *perf_exec_path(void);
 extern void setup_path(void);
 extern int execv_perf_cmd(const char **argv); /* NULL terminated */
 extern int execl_perf_cmd(const char *cmd, ...);
-extern const char *system_path(const char *path);
+/* perf_exec_path and system_path return malloc'd string, caller must free it */
+extern char *perf_exec_path(void);
+extern char *system_path(const char *path);
 
 #endif /* __PERF_EXEC_CMD_H */
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 86c37c4..fa1fc4a 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -159,7 +159,7 @@ void load_command_list(const char *prefix,
 		struct cmdnames *other_cmds)
 {
 	const char *env_path = getenv("PATH");
-	const char *exec_path = perf_exec_path();
+	char *exec_path = perf_exec_path();
 
 	if (exec_path) {
 		list_commands_in_dir(main_cmds, exec_path, prefix);
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		      sizeof(*other_cmds->names), cmdname_compare);
 		uniq(other_cmds);
 	}
+	free(exec_path);
 	exclude_cmds(other_cmds, main_cmds);
 }
 
@@ -203,13 +204,14 @@ void list_commands(const char *title, struct cmdnames *main_cmds,
 			longest = other_cmds->names[i]->len;
 
 	if (main_cmds->cnt) {
-		const char *exec_path = perf_exec_path();
+		char *exec_path = perf_exec_path();
 		printf("available %s in '%s'\n", title, exec_path);
 		printf("----------------");
 		mput_char('-', strlen(title) + strlen(exec_path));
 		putchar('\n');
 		pretty_print_string_list(main_cmds, longest);
 		putchar('\n');
+		free(exec_path);
 	}
 
 	if (other_cmds->cnt) {

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2015-11-23 16:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-18 22:23 [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string Arnaldo Carvalho de Melo
2015-11-19  3:46 ` 平松雅巳 / HIRAMATU,MASAMI
2015-11-19  6:04 ` [PATCH perf/core v2] " Masami Hiramatsu
2015-11-23 16:11   ` [tip:perf/core] perf tools: Make perf_exec_path() always return " tip-bot for Masami Hiramatsu
  -- strict thread matches above, loose matches on Subject: below --
2015-11-18  6:40 [PATCH perf/core 00/13] perf memory/refcnt leak fixes Masami Hiramatsu
2015-11-18  6:40 ` [PATCH perf/core 02/13] perf: Make perf_exec_path always returns malloc'd string Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).