From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752315AbdAaNei (ORCPT ); Tue, 31 Jan 2017 08:34:38 -0500 Received: from mail.kernel.org ([198.145.29.136]:36310 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752110AbdAaNec (ORCPT ); Tue, 31 Jan 2017 08:34:32 -0500 Date: Tue, 31 Jan 2017 10:23:35 -0300 From: Arnaldo Carvalho de Melo To: Taeung Song Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Wang Nan , Jiri Olsa Subject: Re: [PATCH v2 3/4] perf tools: Check NULL after zalloc() and Use zfree() instead of free() in parse-events.c Message-ID: <20170131132335.GE4491@kernel.org> References: <1485862711-20216-1-git-send-email-treeze.taeung@gmail.com> <1485862711-20216-4-git-send-email-treeze.taeung@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1485862711-20216-4-git-send-email-treeze.taeung@gmail.com> X-Url: http://acmel.wordpress.com User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Em Tue, Jan 31, 2017 at 08:38:30PM +0900, Taeung Song escreveu: > 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(). You are doing more than one thing on this patch, don't do that. Please split it for the "no functional changes" parts, i.e. things like - free(path); - path = NULL; + zfree(&path); >>From the ones that _change_ logic, like the first hunk: path = zalloc(sizeof(*path)); + if (!path) + return NULL; path->system = malloc(MAX_EVENT_LENGTH); if (!path->system) { free(path); This one as well changes logic: @@ -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; > Cc: Namhyung Kim > Cc: Jiri Olsa > Signed-off-by: Taeung Song > --- > 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