[PATCH 0/5] perf session: Fill in the missing freeing a session after an error occur

[PATCH 0/5] perf session: Fill in the missing freeing a session after an error occur

[Taeung Song]

In some cases some sessions aren't freed.
For example, a session is allocated and then
if an error occur, just a error value is returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Taeung Song (5):
  perf inject: Fill in the missing freeing a session after an error
    occur
  perf kmem: Fill in the missing freeing a session after an error occur
  perf kvm: Fill in the missing freeing a session after an error occur
  perf mem: Fill in the missing freeing a session after an error occur
  perf report: Fill in the missing freeing a session after an error
    occur

 tools/perf/builtin-inject.c |  7 ++++---
 tools/perf/builtin-kmem.c   |  4 ++--
 tools/perf/builtin-kvm.c    | 16 ++++++++++++----
 tools/perf/builtin-mem.c    | 17 ++++++++++-------
 tools/perf/builtin-report.c |  6 ++++--
 5 files changed, 32 insertions(+), 18 deletions(-)

-- 
1.9.1

[PATCH 1/5] perf inject: Fill in the missing freeing a session after an error occur

[Taeung Song]

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-inject.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 52ec66b..01b0649 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -630,12 +630,13 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (inject.session == NULL)
 		return -1;
 
-	if (symbol__init(&inject.session->header.env) < 0)
-		return -1;
+	ret = symbol__init(&inject.session->header.env);
+	if (ret < 0)
+		goto out_delete;
 
 	ret = __cmd_inject(&inject);
 
+out_delete:
 	perf_session__delete(inject.session);
-
 	return ret;
 }
-- 
1.9.1

Re: [PATCH 1/5] perf inject: Fill in the missing freeing a session after an error occur

[Jiri Olsa]

On Tue, Jun 30, 2015 at 05:15:20PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
> 
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/builtin-inject.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index 52ec66b..01b0649 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -630,12 +630,13 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
>  	if (inject.session == NULL)
>  		return -1;
>  
> -	if (symbol__init(&inject.session->header.env) < 0)
> -		return -1;
> +	ret = symbol__init(&inject.session->header.env);
> +	if (ret < 0)
> +		goto out_delete;
>  
>  	ret = __cmd_inject(&inject);
>  
> +out_delete:
>  	perf_session__delete(inject.session);
> -
>  	return ret;
>  }
> -- 
> 1.9.1
> 

[tip:perf/urgent] perf inject: Fill in the missing session freeing after an error occurs

[tip-bot for Taeung Song]

Commit-ID:  9fedfb0c5b05ae6c315de722a0548bb1f1328bf5
Gitweb:     http://git.kernel.org/tip/9fedfb0c5b05ae6c315de722a0548bb1f1328bf5
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Tue, 30 Jun 2015 17:15:20 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 1 Jul 2015 17:53:49 -0300

perf inject: Fill in the missing session freeing after an error occurs

When an error occur an error value is just returned without freeing the
session. So allocating and freeing session have to be matched as a pair
even if an error occurs.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1435652124-22414-2-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-inject.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index 52ec66b..01b0649 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -630,12 +630,13 @@ int cmd_inject(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (inject.session == NULL)
 		return -1;
 
-	if (symbol__init(&inject.session->header.env) < 0)
-		return -1;
+	ret = symbol__init(&inject.session->header.env);
+	if (ret < 0)
+		goto out_delete;
 
 	ret = __cmd_inject(&inject);
 
+out_delete:
 	perf_session__delete(inject.session);
-
 	return ret;
 }

[PATCH 2/5] perf kmem: Fill in the missing freeing a session after an error occur

[Taeung Song]

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-kmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 950f296..23b1faa 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1916,7 +1916,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
 		if (!perf_evlist__find_tracepoint_by_name(session->evlist,
 							  "kmem:kmalloc")) {
 			pr_err(errmsg, "slab", "slab");
-			return -1;
+			goto out_delete;
 		}
 	}
 
@@ -1927,7 +1927,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
 							     "kmem:mm_page_alloc");
 		if (evsel == NULL) {
 			pr_err(errmsg, "page", "page");
-			return -1;
+			goto out_delete;
 		}
 
 		kmem_page_size = pevent_get_page_size(evsel->tp_format->pevent);
-- 
1.9.1

Re: [PATCH 2/5] perf kmem: Fill in the missing freeing a session after an error occur

[Jiri Olsa]

On Tue, Jun 30, 2015 at 05:15:21PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> 
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/builtin-kmem.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index 950f296..23b1faa 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -1916,7 +1916,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
>  		if (!perf_evlist__find_tracepoint_by_name(session->evlist,
>  							  "kmem:kmalloc")) {
>  			pr_err(errmsg, "slab", "slab");
> -			return -1;
> +			goto out_delete;
>  		}
>  	}
>  
> @@ -1927,7 +1927,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
>  							     "kmem:mm_page_alloc");
>  		if (evsel == NULL) {
>  			pr_err(errmsg, "page", "page");
> -			return -1;
> +			goto out_delete;
>  		}
>  
>  		kmem_page_size = pevent_get_page_size(evsel->tp_format->pevent);
> -- 
> 1.9.1
> 

[tip:perf/urgent] perf kmem: Fill in the missing session freeing after an error occurs

[tip-bot for Taeung Song]

Commit-ID:  249ca1a86067e6a4198f7b2b7e19b505e2f41864
Gitweb:     http://git.kernel.org/tip/249ca1a86067e6a4198f7b2b7e19b505e2f41864
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Tue, 30 Jun 2015 17:15:21 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 1 Jul 2015 17:53:49 -0300

perf kmem: Fill in the missing session freeing after an error occurs

When an error occurs an error value is just returned without freeing the
session. So allocating and freeing session have to be matched as a pair
even if an error occurs.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1435652124-22414-3-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-kmem.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 950f296..23b1faa 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1916,7 +1916,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
 		if (!perf_evlist__find_tracepoint_by_name(session->evlist,
 							  "kmem:kmalloc")) {
 			pr_err(errmsg, "slab", "slab");
-			return -1;
+			goto out_delete;
 		}
 	}
 
@@ -1927,7 +1927,7 @@ int cmd_kmem(int argc, const char **argv, const char *prefix __maybe_unused)
 							     "kmem:mm_page_alloc");
 		if (evsel == NULL) {
 			pr_err(errmsg, "page", "page");
-			return -1;
+			goto out_delete;
 		}
 
 		kmem_page_size = pevent_get_page_size(evsel->tp_format->pevent);

[PATCH 3/5] perf kvm: Fill in the missing freeing a session after an error occur

[Taeung Song]

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-kvm.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 74878cd..5fa96a0 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -1061,8 +1061,10 @@ static int read_events(struct perf_kvm_stat *kvm)
 
 	symbol__init(&kvm->session->header.env);
 
-	if (!perf_session__has_traces(kvm->session, "kvm record"))
-		return -EINVAL;
+	if (!perf_session__has_traces(kvm->session, "kvm record")) {
+		ret = -EINVAL;
+		goto out_delete;
+	}
 
 	/*
 	 * Do not use 'isa' recorded in kvm_exit tracepoint since it is not
@@ -1070,9 +1072,15 @@ static int read_events(struct perf_kvm_stat *kvm)
 	 */
 	ret = cpu_isa_config(kvm);
 	if (ret < 0)
-		return ret;
+		goto out_delete;
 
-	return perf_session__process_events(kvm->session);
+	ret = perf_session__process_events(kvm->session);
+	if (ret < 0)
+		goto out_delete;
+
+out_delete:
+	perf_session__delete(kvm->session);
+	return ret;
 }
 
 static int parse_target_str(struct perf_kvm_stat *kvm)
-- 
1.9.1

Re: [PATCH 3/5] perf kvm: Fill in the missing freeing a session after an error occur

[Jiri Olsa]

On Tue, Jun 30, 2015 at 05:15:22PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
> 
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/builtin-kvm.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 74878cd..5fa96a0 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -1061,8 +1061,10 @@ static int read_events(struct perf_kvm_stat *kvm)
>  
>  	symbol__init(&kvm->session->header.env);
>  
> -	if (!perf_session__has_traces(kvm->session, "kvm record"))
> -		return -EINVAL;
> +	if (!perf_session__has_traces(kvm->session, "kvm record")) {
> +		ret = -EINVAL;
> +		goto out_delete;
> +	}
>  
>  	/*
>  	 * Do not use 'isa' recorded in kvm_exit tracepoint since it is not
> @@ -1070,9 +1072,15 @@ static int read_events(struct perf_kvm_stat *kvm)
>  	 */
>  	ret = cpu_isa_config(kvm);
>  	if (ret < 0)
> -		return ret;
> +		goto out_delete;
>  
> -	return perf_session__process_events(kvm->session);
> +	ret = perf_session__process_events(kvm->session);
> +	if (ret < 0)
> +		goto out_delete;

above 2 lines are not needed.. just fall through

jirka

> +
> +out_delete:
> +	perf_session__delete(kvm->session);
> +	return ret;
>  }
>  
>  static int parse_target_str(struct perf_kvm_stat *kvm)
> -- 
> 1.9.1
> 

[PATCH 4/5] perf mem: Fill in the missing freeing a session after an error occur

[Taeung Song]

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-mem.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
index da2ec06..8b6d473 100644
--- a/tools/perf/builtin-mem.c
+++ b/tools/perf/builtin-mem.c
@@ -135,24 +135,27 @@ static int report_raw_events(struct perf_mem *mem)
 	if (mem->cpu_list) {
 		ret = perf_session__cpu_bitmap(session, mem->cpu_list,
 					       mem->cpu_bitmap);
-		if (ret)
+		if (ret) {
+			ret = err;
 			goto out_delete;
+		}
 	}
 
-	if (symbol__init(&session->header.env) < 0)
-		return -1;
+	ret = symbol__init(&session->header.env);
+	if (ret < 0)
+		goto out_delete;
 
 	printf("# PID, TID, IP, ADDR, LOCAL WEIGHT, DSRC, SYMBOL\n");
 
 	err = perf_session__process_events(session);
 	if (err)
-		return err;
-
-	return 0;
+		ret = err;
+	else
+		ret = 0;
 
 out_delete:
 	perf_session__delete(session);
-	return err;
+	return ret;
 }
 
 static int report_events(int argc, const char **argv, struct perf_mem *mem)
-- 
1.9.1

Re: [PATCH 4/5] perf mem: Fill in the missing freeing a session after an error occur

[Jiri Olsa]

On Tue, Jun 30, 2015 at 05:15:23PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
> 
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/builtin-mem.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/perf/builtin-mem.c b/tools/perf/builtin-mem.c
> index da2ec06..8b6d473 100644
> --- a/tools/perf/builtin-mem.c
> +++ b/tools/perf/builtin-mem.c
> @@ -135,24 +135,27 @@ static int report_raw_events(struct perf_mem *mem)
>  	if (mem->cpu_list) {
>  		ret = perf_session__cpu_bitmap(session, mem->cpu_list,
>  					       mem->cpu_bitmap);
> -		if (ret)
> +		if (ret) {
> +			ret = err;
>  			goto out_delete;
> +		}
>  	}
>  
> -	if (symbol__init(&session->header.env) < 0)
> -		return -1;
> +	ret = symbol__init(&session->header.env);
> +	if (ret < 0)
> +		goto out_delete;
>  
>  	printf("# PID, TID, IP, ADDR, LOCAL WEIGHT, DSRC, SYMBOL\n");
>  
>  	err = perf_session__process_events(session);
>  	if (err)
> -		return err;
> -
> -	return 0;
> +		ret = err;
> +	else
> +		ret = 0;

hum.. how about just:
	ret = perf_session__process_events(session);

and fall through..

also I guess we could get rid of one of those 'ret' and 'err' and use just one?

jirka

>  
>  out_delete:
>  	perf_session__delete(session);
> -	return err;
> +	return ret;
>  }
>  
>  static int report_events(int argc, const char **argv, struct perf_mem *mem)
> -- 
> 1.9.1
> 

[PATCH 5/5] perf report: Fill in the missing freeing a session after an error occur

[Taeung Song]

When an error occur a error value is just returned
without freeing the session. So allocating and freeing
session have to be matched as a pair even if an error occur.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-report.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 32626ea..610d056 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -828,8 +828,10 @@ repeat:
 	if (report.header || report.header_only) {
 		perf_session__fprintf_info(session, stdout,
 					   report.show_full_info);
-		if (report.header_only)
-			return 0;
+		if (report.header_only) {
+			ret = 0;
+			goto error;
+		}
 	} else if (use_browser == 0) {
 		fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
 		      stdout);
-- 
1.9.1

Re: [PATCH 5/5] perf report: Fill in the missing freeing a session after an error occur

[Jiri Olsa]

On Tue, Jun 30, 2015 at 05:15:24PM +0900, Taeung Song wrote:
> When an error occur a error value is just returned
> without freeing the session. So allocating and freeing
> session have to be matched as a pair even if an error occur.
> 
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

> ---
>  tools/perf/builtin-report.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 32626ea..610d056 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -828,8 +828,10 @@ repeat:
>  	if (report.header || report.header_only) {
>  		perf_session__fprintf_info(session, stdout,
>  					   report.show_full_info);
> -		if (report.header_only)
> -			return 0;
> +		if (report.header_only) {
> +			ret = 0;
> +			goto error;
> +		}
>  	} else if (use_browser == 0) {
>  		fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
>  		      stdout);
> -- 
> 1.9.1
> 

[tip:perf/urgent] perf report: Fill in the missing session freeing after an error occurs

[tip-bot for Taeung Song]

Commit-ID:  07a716fff25b826461baa2a07faa2df8c171f220
Gitweb:     http://git.kernel.org/tip/07a716fff25b826461baa2a07faa2df8c171f220
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Tue, 30 Jun 2015 17:15:24 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 1 Jul 2015 17:53:49 -0300

perf report: Fill in the missing session freeing after an error occurs

When an error occurs an error value is just returned without freeing the
session. So allocating and freeing session have to be matched as a pair
even if an error occurs.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: http://lkml.kernel.org/r/1435652124-22414-6-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-report.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 348bed4..95a4771 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -839,8 +839,10 @@ repeat:
 	if (report.header || report.header_only) {
 		perf_session__fprintf_info(session, stdout,
 					   report.show_full_info);
-		if (report.header_only)
-			return 0;
+		if (report.header_only) {
+			ret = 0;
+			goto error;
+		}
 	} else if (use_browser == 0) {
 		fputs("# To display the perf.data header info, please use --header/--header-only options.\n#\n",
 		      stdout);