[PATCH] perf tool: fix trivial memory leak while calling system_path()

[PATCH] perf tool: fix trivial memory leak while calling system_path()

[liang xie]

A trivial memory leak fix while calling system_path

Signed-off-by: Liang Xie <xieliang@xiaomi.com>
---
 tools/perf/util/exec_cmd.c |    6 ++++--
 tools/perf/util/help.c     |    1 +
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..790cc95 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -83,8 +83,8 @@ void setup_path(void)
 {
 	const char *old_path = getenv("PATH");
 	struct strbuf new_path = STRBUF_INIT;
-
-	add_path(&new_path, perf_exec_path());
+	const char *exec_path = perf_exec_path();
+	add_path(&new_path, exec_path);
 	add_path(&new_path, argv0_path);

 	if (old_path)
@@ -95,6 +95,8 @@ void setup_path(void)
 	setenv("PATH", new_path.buf, 1);

 	strbuf_release(&new_path);
+	if (exec_path)
+		free((void *)exec_path);
 }

 static const char **prepare_perf_cmd(const char **argv)
diff --git a/tools/perf/util/help.c b/tools/perf/util/help.c
index 6f2975a..798f66d 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -187,6 +187,7 @@ void load_command_list(const char *prefix,
 		uniq(other_cmds);
 	}
 	exclude_cmds(other_cmds, main_cmds);
+	free((void *)exec_path);
 }

 void list_commands(const char *title, struct cmdnames *main_cmds,
-- 
1.7.1

Re: [PATCH] perf tool: fix trivial memory leak while calling system_path()

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1107 bytes --]

Hi,

On Wed, Sep 05, 2012 at 09:48:54PM +0800, liang xie wrote:
> A trivial memory leak fix while calling system_path
> 
> Signed-off-by: Liang Xie <xieliang@xiaomi.com>
> ---
>  tools/perf/util/exec_cmd.c |    6 ++++--
>  tools/perf/util/help.c     |    1 +
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
> index 7adf4ad..790cc95 100644
> --- a/tools/perf/util/exec_cmd.c
> +++ b/tools/perf/util/exec_cmd.c
> @@ -83,8 +83,8 @@ void setup_path(void)
>  {
>  	const char *old_path = getenv("PATH");
>  	struct strbuf new_path = STRBUF_INIT;
> -
> -	add_path(&new_path, perf_exec_path());
> +	const char *exec_path = perf_exec_path();
> +	add_path(&new_path, exec_path);
>  	add_path(&new_path, argv0_path);
> 
>  	if (old_path)
> @@ -95,6 +95,8 @@ void setup_path(void)
>  	setenv("PATH", new_path.buf, 1);
> 
>  	strbuf_release(&new_path);
> +	if (exec_path)

free(NULL) is safe, the check isn't needed.

> +		free((void *)exec_path);

this cast doesn't look necessary either.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] perf tool: fix trivial memory leak while calling system_path()

[liang xie]

Hi Felipe,

Thanks for reviewing! yeh, the check could be removed safely .
For casting, it'll complain like this if removed:
cc1: warnings being treated as errors
util/help.c: In function ‘load_command_list’:
util/help.c:190: error: passing argument 1 of ‘free’ discards
qualifiers from pointer target type
/usr/include/stdlib.h:488: note: expected ‘void *’ but argument is of
type ‘const char *’
make: *** [util/help.o] Error 1

Best Regards,

On Wed, Sep 5, 2012 at 9:49 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Wed, Sep 05, 2012 at 09:48:54PM +0800, liang xie wrote:
>> A trivial memory leak fix while calling system_path
>>
>> Signed-off-by: Liang Xie <xieliang@xiaomi.com>
>> ---
>>  tools/perf/util/exec_cmd.c |    6 ++++--
>>  tools/perf/util/help.c     |    1 +
>>  2 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
>> index 7adf4ad..790cc95 100644
>> --- a/tools/perf/util/exec_cmd.c
>> +++ b/tools/perf/util/exec_cmd.c
>> @@ -83,8 +83,8 @@ void setup_path(void)
>>  {
>>       const char *old_path = getenv("PATH");
>>       struct strbuf new_path = STRBUF_INIT;
>> -
>> -     add_path(&new_path, perf_exec_path());
>> +     const char *exec_path = perf_exec_path();
>> +     add_path(&new_path, exec_path);
>>       add_path(&new_path, argv0_path);
>>
>>       if (old_path)
>> @@ -95,6 +95,8 @@ void setup_path(void)
>>       setenv("PATH", new_path.buf, 1);
>>
>>       strbuf_release(&new_path);
>> +     if (exec_path)
>
> free(NULL) is safe, the check isn't needed.
>
>> +             free((void *)exec_path);
>
> this cast doesn't look necessary either.
>
> --
> balbi