Messages in this thread | | | Subject | Re: [PATCH v2 3/4] perf tools: Check NULL after zalloc() and Use zfree() instead of free() in parse-events.c | From | Taeung Song <> | Date | Wed, 1 Feb 2017 17:04:08 +0900 |
| |
Sorry I'm late.. I got it! I'll send v3 with changed patches !!
Thanks, Taeung
On 01/31/2017 10:23 PM, Arnaldo Carvalho de Melo wrote: > 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 <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
| |