All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c
@ 2017-01-30  5:23 Taeung Song
  2017-01-30  5:23 ` [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output() Taeung Song
  2017-01-30  9:01 ` [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Jiri Olsa
  0 siblings, 2 replies; 7+ messages in thread
From: Taeung Song @ 2017-01-30  5:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Wang Nan, Taeung Song, Jiri Olsa

Currently there are several parts not checking NULL
after allocating with zalloc() or asigning NULL value
to a pointer variable after doing free().

So I fill in code checking NULL and
use zfree() instead of free().

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/parse-events.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 3c876b8..87a3e5a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -211,6 +211,8 @@ struct tracepoint_path *tracepoint_id_to_path(u64 config)
 				closedir(evt_dir);
 				closedir(sys_dir);
 				path = zalloc(sizeof(*path));
+				if (!path)
+					return NULL;
 				path->system = malloc(MAX_EVENT_LENGTH);
 				if (!path->system) {
 					free(path);
@@ -252,8 +254,7 @@ struct tracepoint_path *tracepoint_name_to_path(const char *name)
 	if (path->system == NULL || path->name == NULL) {
 		zfree(&path->system);
 		zfree(&path->name);
-		free(path);
-		path = NULL;
+		zfree(&path);
 	}
 
 	return path;
@@ -1477,10 +1478,9 @@ static void perf_pmu__parse_cleanup(void)
 
 		for (i = 0; i < perf_pmu_events_list_num; i++) {
 			p = perf_pmu_events_list + i;
-			free(p->symbol);
+			zfree(&p->symbol);
 		}
-		free(perf_pmu_events_list);
-		perf_pmu_events_list = NULL;
+		zfree(&perf_pmu_events_list);
 		perf_pmu_events_list_num = 0;
 	}
 }
@@ -1563,7 +1563,7 @@ perf_pmu__parse_check(const char *name)
 	r = bsearch(&p, perf_pmu_events_list,
 			(size_t) perf_pmu_events_list_num,
 			sizeof(struct perf_pmu_event_symbol), comp_pmu);
-	free(p.symbol);
+	zfree(&p.symbol);
 	return r ? r->type : PMU_EVENT_SYMBOL_ERR;
 }
 
@@ -1710,8 +1710,8 @@ static void parse_events_print_error(struct parse_events_error *err,
 		fprintf(stderr, "%*s\\___ %s\n", idx + 1, "", err->str);
 		if (err->help)
 			fprintf(stderr, "\n%s\n", err->help);
-		free(err->str);
-		free(err->help);
+		zfree(&err->str);
+		zfree(&err->help);
 	}
 
 	fprintf(stderr, "Run 'perf list' for a list of valid events\n");
@@ -2406,7 +2406,7 @@ void parse_events_terms__purge(struct list_head *terms)
 
 	list_for_each_entry_safe(term, h, terms, list) {
 		if (term->array.nr_ranges)
-			free(term->array.ranges);
+			zfree(&term->array.ranges);
 		list_del_init(&term->list);
 		free(term);
 	}
@@ -2422,7 +2422,7 @@ void parse_events_terms__delete(struct list_head *terms)
 
 void parse_events__clear_array(struct parse_events_array *a)
 {
-	free(a->ranges);
+	zfree(&a->ranges);
 }
 
 void parse_events_evlist_error(struct parse_events_evlist *data,
-- 
2.7.4

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

* [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output()
  2017-01-30  5:23 [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Taeung Song
@ 2017-01-30  5:23 ` Taeung Song
  2017-01-30  8:55   ` Jiri Olsa
  2017-01-30  9:01 ` [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Jiri Olsa
  1 sibling, 1 reply; 7+ messages in thread
From: Taeung Song @ 2017-01-30  5:23 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Wang Nan, Taeung Song, Jiri Olsa

If 'evsel' is NULL, in perf_evsel__is_bpf_output()
NULL pointer error can happen so check it.

Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/evsel.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 04e536a..b77da72 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -242,8 +242,10 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
 {
 	struct perf_evsel *evsel = zalloc(perf_evsel__object.size);
 
-	if (evsel != NULL)
-		perf_evsel__init(evsel, attr, idx);
+	if (!evsel)
+		return NULL;
+
+	perf_evsel__init(evsel, attr, idx);
 
 	if (perf_evsel__is_bpf_output(evsel)) {
 		evsel->attr.sample_type |= (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME |
-- 
2.7.4

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

* Re: [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output()
  2017-01-30  5:23 ` [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output() Taeung Song
@ 2017-01-30  8:55   ` Jiri Olsa
  2017-01-30 10:26     ` Taeung Song
  0 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2017-01-30  8:55 UTC (permalink / raw)
  To: Taeung Song
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim,
	Ingo Molnar, Peter Zijlstra, Wang Nan

On Mon, Jan 30, 2017 at 02:23:39PM +0900, Taeung Song wrote:
> If 'evsel' is NULL, in perf_evsel__is_bpf_output()
> NULL pointer error can happen so check it.
> 
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/evsel.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 04e536a..b77da72 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -242,8 +242,10 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
>  {
>  	struct perf_evsel *evsel = zalloc(perf_evsel__object.size);
>  
> -	if (evsel != NULL)
> -		perf_evsel__init(evsel, attr, idx);
> +	if (!evsel)
> +		return NULL;
> +
> +	perf_evsel__init(evsel, attr, idx);

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

thanks,
jirka

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

* Re: [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c
  2017-01-30  5:23 [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Taeung Song
  2017-01-30  5:23 ` [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output() Taeung Song
@ 2017-01-30  9:01 ` Jiri Olsa
  2017-01-30 10:37   ` Taeung Song
  1 sibling, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2017-01-30  9:01 UTC (permalink / raw)
  To: Taeung Song
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim,
	Ingo Molnar, Peter Zijlstra, Wang Nan

On Mon, Jan 30, 2017 at 02:23:38PM +0900, Taeung Song wrote:
> Currently there are several parts not checking NULL
> after allocating with zalloc() or asigning NULL value
> to a pointer variable after doing free().
> 
> So I fill in code checking NULL and
> use zfree() instead of free().

can't see directly reasons for zfree usage,
but it looks reasonable.. do you have any
crash reports due to missing zfree?

thanks,
jirka

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

* Re: [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output()
  2017-01-30  8:55   ` Jiri Olsa
@ 2017-01-30 10:26     ` Taeung Song
  0 siblings, 0 replies; 7+ messages in thread
From: Taeung Song @ 2017-01-30 10:26 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim,
	Ingo Molnar, Peter Zijlstra, Wang Nan

Hi, jirka :)

Thank you!
Taeung

On 01/30/2017 05:55 PM, Jiri Olsa wrote:
> On Mon, Jan 30, 2017 at 02:23:39PM +0900, Taeung Song wrote:
>> If 'evsel' is NULL, in perf_evsel__is_bpf_output()
>> NULL pointer error can happen so check it.
>>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>  tools/perf/util/evsel.c | 6 ++++--
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>> index 04e536a..b77da72 100644
>> --- a/tools/perf/util/evsel.c
>> +++ b/tools/perf/util/evsel.c
>> @@ -242,8 +242,10 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx)
>>  {
>>  	struct perf_evsel *evsel = zalloc(perf_evsel__object.size);
>>
>> -	if (evsel != NULL)
>> -		perf_evsel__init(evsel, attr, idx);
>> +	if (!evsel)
>> +		return NULL;
>> +
>> +	perf_evsel__init(evsel, attr, idx);
>
> Acked-by: Jiri Olsa <jolsa@kernel.org>
>
> thanks,
> jirka
>

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

* Re: [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c
  2017-01-30  9:01 ` [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Jiri Olsa
@ 2017-01-30 10:37   ` Taeung Song
  2017-01-30 12:39     ` Jiri Olsa
  0 siblings, 1 reply; 7+ messages in thread
From: Taeung Song @ 2017-01-30 10:37 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim,
	Ingo Molnar, Peter Zijlstra, Wang Nan



On 01/30/2017 06:01 PM, Jiri Olsa wrote:
> On Mon, Jan 30, 2017 at 02:23:38PM +0900, Taeung Song wrote:
>> Currently there are several parts not checking NULL
>> after allocating with zalloc() or asigning NULL value
>> to a pointer variable after doing free().
>>
>> So I fill in code checking NULL and
>> use zfree() instead of free().
>
> can't see directly reasons for zfree usage,
> but it looks reasonable.. do you have any
> crash reports due to missing zfree?

No, Just I read source code util/parse-events.c
And I found several insufficiency, shortcoming
not checking NULL or assigning NULL value to a pointer
variable after free().
So, I think we can use zfree() insteadof free()+assigning NULL.

Change commit message to be more appropriate ?

Thanks,
Taeung

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

* Re: [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c
  2017-01-30 10:37   ` Taeung Song
@ 2017-01-30 12:39     ` Jiri Olsa
  0 siblings, 0 replies; 7+ messages in thread
From: Jiri Olsa @ 2017-01-30 12:39 UTC (permalink / raw)
  To: Taeung Song
  Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Namhyung Kim,
	Ingo Molnar, Peter Zijlstra, Wang Nan

On Mon, Jan 30, 2017 at 07:37:58PM +0900, Taeung Song wrote:
> 
> 
> On 01/30/2017 06:01 PM, Jiri Olsa wrote:
> > On Mon, Jan 30, 2017 at 02:23:38PM +0900, Taeung Song wrote:
> > > Currently there are several parts not checking NULL
> > > after allocating with zalloc() or asigning NULL value
> > > to a pointer variable after doing free().
> > > 
> > > So I fill in code checking NULL and
> > > use zfree() instead of free().
> > 
> > can't see directly reasons for zfree usage,
> > but it looks reasonable.. do you have any
> > crash reports due to missing zfree?
> 
> No, Just I read source code util/parse-events.c
> And I found several insufficiency, shortcoming
> not checking NULL or assigning NULL value to a pointer
> variable after free().
> So, I think we can use zfree() insteadof free()+assigning NULL.

ok

> 
> Change commit message to be more appropriate ?

that's always good idea ;-)

thanks,
jirka

> 
> Thanks,
> Taeung
> 

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

end of thread, other threads:[~2017-01-30 12:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-30  5:23 [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Taeung Song
2017-01-30  5:23 ` [PATCH 2/2] perf evsel: Check for NULL before perf_evsel__is_bpf_output() Taeung Song
2017-01-30  8:55   ` Jiri Olsa
2017-01-30 10:26     ` Taeung Song
2017-01-30  9:01 ` [PATCH 1/2] perf tools: Use zfree() instead of free() in parse-events.c Jiri Olsa
2017-01-30 10:37   ` Taeung Song
2017-01-30 12:39     ` Jiri Olsa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.