[<prev] [next>] [day] [month] [year] [list]
Message-ID: <CADu=CFoVkk3Vujf3bqJ6SALzpJPC_7uq-rYexer5ESEABd_d4Q@mail.gmail.com>
Date: Mon, 17 Sep 2012 16:47:08 +0800
From: liang xie <xieliang007@...il.com>
To: linux-perf-users@...r.kernel.org
Cc: acme@...stprotocols.net, namhyung@...il.com, balbi@...com,
a.p.zijlstra@...llo.nl, mingo@...e.hu, paulus@...ba.org,
linux-kernel@...r.kernel.org
Subject: [PATCH v4] 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&perf_exec_path always return dynamically
allocated string
Since v3: check strdup() per Namhyung's comments
Signed-off-by: xieliang <xieliang@...omi.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 | 42 +++++++++++++++++++++++++++++-------------
tools/perf/util/exec_cmd.h | 4 ++--
tools/perf/util/help.c | 6 ++++--
7 files changed, 65 insertions(+), 36 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 *envchanged)
if (*cmd == '=')
perf_set_argv_exec_path(cmd + 1);
else {
- puts(perf_exec_path());
+ char *exec_path = perf_exec_path();
+ puts(exec_path);
+ free(exec_path);
exit(0);
}
} else if (!strcmp(cmd, "--html-path")) {
- puts(system_path(PERF_HTML_PATH));
+ char *html_path = system_path(PERF_HTML_PATH);
+ puts(html_path);
+ free(html_path);
exit(0);
} else if (!strcmp(cmd, "-p") || !strcmp(cmd, "--paginate")) {
use_pager = 1;
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 6faa3a1..7fb00e2 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -375,14 +375,6 @@ static int perf_config_from_file(config_fn_t fn,
const char *filename, void *dat
return ret;
}
-static const char *perf_etc_perfconfig(void)
-{
- static const char *system_wide;
- if (!system_wide)
- system_wide = system_path(ETC_PERFCONFIG);
- return system_wide;
-}
-
static int perf_env_bool(const char *k, int def)
{
const char *v = getenv(k);
@@ -403,15 +395,17 @@ int perf_config(config_fn_t fn, void *data)
{
int ret = 0, found = 0;
const char *home = NULL;
+ char *etc_perfconfig = NULL;
/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */
if (config_exclusive_filename)
return perf_config_from_file(fn, config_exclusive_filename, data);
- if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK)) {
- ret += perf_config_from_file(fn, perf_etc_perfconfig(),
- data);
+ etc_perfconfig = system_path(ETC_PERFCONFIG);
+ if (perf_config_system() && !access(etc_perfconfig, R_OK)) {
+ ret += perf_config_from_file(fn, etc_perfconfig, data);
found += 1;
}
+ free(etc_perfconfig);
home = getenv("HOME");
if (perf_config_global() && home) {
diff --git a/tools/perf/util/exec_cmd.c b/tools/perf/util/exec_cmd.c
index 7adf4ad..7e5c7c2 100644
--- a/tools/perf/util/exec_cmd.c
+++ b/tools/perf/util/exec_cmd.c
@@ -9,17 +9,23 @@
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;
+ if (is_absolute_path(path)) {
+ new_path = strdup(path);
+ if (!new_path)
+ die("malloc");
+ return new_path;
+ }
+
+ new_path = malloc((strlen(PREFIX) + strlen(path) + 2));
+ if (!new_path)
+ die("malloc");
- strbuf_addf(&d, "%s/%s", prefix, path);
- path = strbuf_detach(&d, NULL);
- return path;
+ sprintf(new_path, "%s/%s", PREFIX, path);
+ return new_path;
}
const char *perf_extract_argv0_path(const char *argv0)
@@ -52,16 +58,24 @@ 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 *path;
- if (argv_exec_path)
- return argv_exec_path;
+ if (argv_exec_path) {
+ path = strdup(argv_exec_path);
+ if (!path)
+ die("malloc");
+ return path;
+ }
env = getenv(EXEC_PATH_ENVIRONMENT);
if (env && *env) {
- return env;
+ path = strdup(env);
+ if (!path)
+ die("malloc");
+ return path;
}
return system_path(PERF_EXEC_PATH);
@@ -83,8 +97,9 @@ void setup_path(void)
{
const char *old_path = getenv("PATH");
struct strbuf new_path = STRBUF_INIT;
+ char *exec_path = perf_exec_path();
- add_path(&new_path, perf_exec_path());
+ add_path(&new_path, exec_path);
add_path(&new_path, argv0_path);
if (old_path)
@@ -95,6 +110,7 @@ void setup_path(void)
setenv("PATH", new_path.buf, 1);
strbuf_release(&new_path);
+ free(exec_path);
}
static const char **prepare_perf_cmd(const char **argv)
diff --git a/tools/perf/util/exec_cmd.h b/tools/perf/util/exec_cmd.h
index bc4b915..8bfde93 100644
--- a/tools/perf/util/exec_cmd.h
+++ b/tools/perf/util/exec_cmd.h
@@ -3,10 +3,10 @@
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 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);
+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 6f2975a..ba1de76 100644
--- a/tools/perf/util/help.c
+++ b/tools/perf/util/help.c
@@ -158,7 +158,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,
uniq(other_cmds);
}
exclude_cmds(other_cmds, main_cmds);
+ free(exec_path);
}
void list_commands(const char *title, struct cmdnames *main_cmds,
@@ -202,13 +203,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) {
--
1.7.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists