Re: [PATCH v3] memory leak fix while calling system_path
Hi,
On Fri, 14 Sep 2012 11:14:13 +0800, liang xie wrote:
> memory leak fix while calling system_path
>
> Since v1: Remove an unnecessary null pointer check per Felipe's comments
> Since v2: Make system_path_exec_path always return dynamically
> allocated string
>
> Signed-off-by: xieliang
> ---
> tools/perf/builtin-help.c | 12 +---
> tools/perf/builtin-script.c | 13 ++---
> tools/perf/perf.c |8 ++--
> tools/perf/util/config.c| 16 +---
> tools/perf/util/exec_cmd.c | 26 +++---
> tools/perf/util/exec_cmd.h |4 ++--
> tools/perf/util/help.c |6 --
> 7 files changed, 51 insertions(+), 34 deletions(-)
>
> diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
> index 6d5a8a7..180a5bd 100644
> --- a/tools/perf/builtin-help.c
> +++ b/tools/perf/builtin-help.c
> @@ -321,12 +321,13 @@ static void setup_man_path(void)
> {
> struct strbuf new_path = STRBUF_INIT;
> const char *old_path = getenv("MANPATH");
> + char *man_path = system_path(PERF_MAN_PATH);
I think the return value of system_path and perf_exec_path should be
checked since they can return NULL as strdup() does.
[snip]
> diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
> index 7adf4ad..1ace941 100644
> --- a/tools/perf/util/exec_cmd.c
> +++ b/tools/perf/util/exec_cmd.c
> @@ -9,17 +9,19 @@
> 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;
> + char *new_path = NULL;
>
> if (is_absolute_path(path))
> - return path;
> + return strdup(path);
>
> - strbuf_addf(, "%s/%s", prefix, path);
> - path = strbuf_detach(, NULL);
> - return path;
> + new_path = malloc((strlen(PREFIX) + strlen(path) + 2));
> + if (!new_path)
> + die("malloc");
> +
> + sprintf(new_path, "%s/%s", PREFIX, path);
> + return new_path;
AFAIK strbuf allocates memory internally, so why this code is needed?
Thanks,
Namhyung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] memory leak fix while calling system_path
Hi,
On Fri, 14 Sep 2012 11:14:13 +0800, liang xie wrote:
memory leak fix while calling system_path
Since v1: Remove an unnecessary null pointer check per Felipe's comments
Since v2: Make system_pathperf_exec_path always return dynamically
allocated string
Signed-off-by: xieliang xieli...@xiaomi.com
---
tools/perf/builtin-help.c | 12 +---
tools/perf/builtin-script.c | 13 ++---
tools/perf/perf.c |8 ++--
tools/perf/util/config.c| 16 +---
tools/perf/util/exec_cmd.c | 26 +++---
tools/perf/util/exec_cmd.h |4 ++--
tools/perf/util/help.c |6 --
7 files changed, 51 insertions(+), 34 deletions(-)
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 6d5a8a7..180a5bd 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -321,12 +321,13 @@ static void setup_man_path(void)
{
struct strbuf new_path = STRBUF_INIT;
const char *old_path = getenv(MANPATH);
+ char *man_path = system_path(PERF_MAN_PATH);
I think the return value of system_path and perf_exec_path should be
checked since they can return NULL as strdup() does.
[snip]
diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..1ace941 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,19 @@
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;
+ char *new_path = NULL;
if (is_absolute_path(path))
- return path;
+ return strdup(path);
- strbuf_addf(d, %s/%s, prefix, path);
- path = strbuf_detach(d, NULL);
- return path;
+ new_path = malloc((strlen(PREFIX) + strlen(path) + 2));
+ if (!new_path)
+ die(malloc);
+
+ sprintf(new_path, %s/%s, PREFIX, path);
+ return new_path;
AFAIK strbuf allocates memory internally, so why this code is needed?
Thanks,
Namhyung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH v3] memory leak fix while calling system_path
memory leak fix while calling system_path
Since v1: Remove an unnecessary null pointer check per Felipe's comments
Since v2: Make system_path_exec_path always return dynamically
allocated string
Signed-off-by: xieliang
---
tools/perf/builtin-help.c | 12 +---
tools/perf/builtin-script.c | 13 ++---
tools/perf/perf.c |8 ++--
tools/perf/util/config.c| 16 +---
tools/perf/util/exec_cmd.c | 26 +++---
tools/perf/util/exec_cmd.h |4 ++--
tools/perf/util/help.c |6 --
7 files changed, 51 insertions(+), 34 deletions(-)
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 6d5a8a7..180a5bd 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -321,12 +321,13 @@ static void setup_man_path(void)
{
struct strbuf new_path = STRBUF_INIT;
const char *old_path = getenv("MANPATH");
+ char *man_path = system_path(PERF_MAN_PATH);
/* We should always put ':' after our path. If there is no
* old_path, the ':' at the end will let 'man' to try
* system-wide paths after ours to find the manual page. If
* there is old_path, we need ':' as delimiter. */
- strbuf_addstr(_path, system_path(PERF_MAN_PATH));
+ strbuf_addstr(_path, man_path);
strbuf_addch(_path, ':');
if (old_path)
strbuf_addstr(_path, old_path);
@@ -334,6 +335,7 @@ static void setup_man_path(void)
setenv("MANPATH", new_path.buf, 1);
strbuf_release(_path);
+ free(man_path);
}
static void exec_viewer(const char *name, const char *page)
@@ -371,14 +373,16 @@ static void show_man_page(const char *perf_cmd)
static void show_info_page(const char *perf_cmd)
{
const char *page = cmd_to_page(perf_cmd);
- setenv("INFOPATH", system_path(PERF_INFO_PATH), 1);
+ char *info_path = system_path(PERF_INFO_PATH);
+ setenv("INFOPATH", info_path, 1);
execlp("info", "info", "perfman", page, NULL);
+ free(info_path);
}
static void get_html_page_path(struct strbuf *page_path, const char *page)
{
struct stat st;
- const char *html_path = system_path(PERF_HTML_PATH);
+ char *html_path = system_path(PERF_HTML_PATH);
/* Check that we have a perf documentation directory. */
if (stat(mkpath("%s/perf.html", html_path), )
@@ -387,6 +391,7 @@ static void get_html_page_path(struct strbuf
*page_path, const char *page)
strbuf_init(page_path, 0);
strbuf_addf(page_path, "%s/%s.html", html_path, page);
+ free(html_path);
}
/*
@@ -409,6 +414,7 @@ static void show_html_page(const char *perf_cmd)
get_html_page_path(_path, page);
open_html(page_path.buf);
+ strbuf_release(_path);
}
int cmd_help(int argc, const char **argv, const char *prefix __used)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 1e60ab7..528b786 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1021,8 +1021,10 @@ static int list_available_scripts(const struct
option *opt __used,
struct script_desc *desc;
char first_half[BUFSIZ];
char *script_root;
+ char *exec_path = perf_exec_path();
- snprintf(scripts_path, MAXPATHLEN, "%s/scripts", perf_exec_path());
+ snprintf(scripts_path, MAXPATHLEN, "%s/scripts", exec_path);
+ free(exec_path);
scripts_dir = opendir(scripts_path);
if (!scripts_dir)
@@ -1066,8 +1068,10 @@ static char *get_script_path(const char
*script_root, const char *suffix)
DIR *scripts_dir, *lang_dir;
char lang_path[MAXPATHLEN];
char *__script_root;
+ char *exec_path = perf_exec_path();
- snprintf(scripts_path, MAXPATHLEN, "%s/scripts", perf_exec_path());
+ snprintf(scripts_path, MAXPATHLEN, "%s/scripts", exec_path);
+ free(exec_path);
scripts_dir = opendir(scripts_path);
if (!scripts_dir)
@@ -1199,6 +1203,7 @@ int cmd_script(int argc, const char **argv,
const char *prefix __used)
{
char *rec_script_path = NULL;
char *rep_script_path = NULL;
+ char *exec_path = NULL;
struct perf_session *session;
char *script_path = NULL;
const char **__argv;
@@ -1226,7 +1231,9 @@ int cmd_script(int argc, const char **argv,
const char *prefix __used)
}
/* make sure PERF_EXEC_PATH is set for scripts */
- perf_set_argv_exec_path(perf_exec_path());
+ exec_path = perf_exec_path();
+ perf_set_argv_exec_path(exec_path);
+ free(exec_path);
if (argc && !script_name && !rec_script_path && !rep_script_path) {
int live_pipe[2];
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 2b2e225..2198725 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -104,11 +104,15 @@ static int handle_options(const char ***argv,
int *argc, int *envchanged)
[PATCH v3] memory leak fix while calling system_path
memory leak fix while calling system_path
Since v1: Remove an unnecessary null pointer check per Felipe's comments
Since v2: Make system_pathperf_exec_path always return dynamically
allocated string
Signed-off-by: xieliang xieli...@xiaomi.com
---
tools/perf/builtin-help.c | 12 +---
tools/perf/builtin-script.c | 13 ++---
tools/perf/perf.c |8 ++--
tools/perf/util/config.c| 16 +---
tools/perf/util/exec_cmd.c | 26 +++---
tools/perf/util/exec_cmd.h |4 ++--
tools/perf/util/help.c |6 --
7 files changed, 51 insertions(+), 34 deletions(-)
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 6d5a8a7..180a5bd 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -321,12 +321,13 @@ static void setup_man_path(void)
{
struct strbuf new_path = STRBUF_INIT;
const char *old_path = getenv(MANPATH);
+ char *man_path = system_path(PERF_MAN_PATH);
/* We should always put ':' after our path. If there is no
* old_path, the ':' at the end will let 'man' to try
* system-wide paths after ours to find the manual page. If
* there is old_path, we need ':' as delimiter. */
- strbuf_addstr(new_path, system_path(PERF_MAN_PATH));
+ strbuf_addstr(new_path, man_path);
strbuf_addch(new_path, ':');
if (old_path)
strbuf_addstr(new_path, old_path);
@@ -334,6 +335,7 @@ static void setup_man_path(void)
setenv(MANPATH, new_path.buf, 1);
strbuf_release(new_path);
+ free(man_path);
}
static void exec_viewer(const char *name, const char *page)
@@ -371,14 +373,16 @@ static void show_man_page(const char *perf_cmd)
static void show_info_page(const char *perf_cmd)
{
const char *page = cmd_to_page(perf_cmd);
- setenv(INFOPATH, system_path(PERF_INFO_PATH), 1);
+ char *info_path = system_path(PERF_INFO_PATH);
+ setenv(INFOPATH, info_path, 1);
execlp(info, info, perfman, page, NULL);
+ free(info_path);
}
static void get_html_page_path(struct strbuf *page_path, const char *page)
{
struct stat st;
- const char *html_path = system_path(PERF_HTML_PATH);
+ char *html_path = system_path(PERF_HTML_PATH);
/* Check that we have a perf documentation directory. */
if (stat(mkpath(%s/perf.html, html_path), st)
@@ -387,6 +391,7 @@ static void get_html_page_path(struct strbuf
*page_path, const char *page)
strbuf_init(page_path, 0);
strbuf_addf(page_path, %s/%s.html, html_path, page);
+ free(html_path);
}
/*
@@ -409,6 +414,7 @@ static void show_html_page(const char *perf_cmd)
get_html_page_path(page_path, page);
open_html(page_path.buf);
+ strbuf_release(page_path);
}
int cmd_help(int argc, const char **argv, const char *prefix __used)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 1e60ab7..528b786 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1021,8 +1021,10 @@ static int list_available_scripts(const struct
option *opt __used,
struct script_desc *desc;
char first_half[BUFSIZ];
char *script_root;
+ char *exec_path = perf_exec_path();
- snprintf(scripts_path, MAXPATHLEN, %s/scripts, perf_exec_path());
+ snprintf(scripts_path, MAXPATHLEN, %s/scripts, exec_path);
+ free(exec_path);
scripts_dir = opendir(scripts_path);
if (!scripts_dir)
@@ -1066,8 +1068,10 @@ static char *get_script_path(const char
*script_root, const char *suffix)
DIR *scripts_dir, *lang_dir;
char lang_path[MAXPATHLEN];
char *__script_root;
+ char *exec_path = perf_exec_path();
- snprintf(scripts_path, MAXPATHLEN, %s/scripts, perf_exec_path());
+ snprintf(scripts_path, MAXPATHLEN, %s/scripts, exec_path);
+ free(exec_path);
scripts_dir = opendir(scripts_path);
if (!scripts_dir)
@@ -1199,6 +1203,7 @@ int cmd_script(int argc, const char **argv,
const char *prefix __used)
{
char *rec_script_path = NULL;
char *rep_script_path = NULL;
+ char *exec_path = NULL;
struct perf_session *session;
char *script_path = NULL;
const char **__argv;
@@ -1226,7 +1231,9 @@ int cmd_script(int argc, const char **argv,
const char *prefix __used)
}
/* make sure PERF_EXEC_PATH is set for scripts */
- perf_set_argv_exec_path(perf_exec_path());
+ exec_path = perf_exec_path();
+ perf_set_argv_exec_path(exec_path);
+ free(exec_path);
if (argc !script_name !rec_script_path !rep_script_path) {
int live_pipe[2];
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 2b2e225..2198725 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -104,11 +104,15 @@ static int handle_options(const char ***argv,
int *argc, int
