Messages in this thread | | | Date | Mon, 01 Jul 2013 11:01:24 +0300 | From | Adrian Hunter <> | Subject | Re: [PATCH V3 06/15] perf tools: fix parse_events_terms() freeing local variable on error path |
| |
On 28/06/13 20:19, David Ahern wrote: > On 6/28/13 2:43 AM, Adrian Hunter wrote: >> The list_head is on the stack, so just free the rest of the list. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> >> --- >> tools/perf/util/parse-events.c | 7 ++++++- >> tools/perf/util/parse-events.h | 1 + >> tools/perf/util/pmu.c | 2 +- >> 3 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c >> index 995fc25..d9cb055 100644 >> --- a/tools/perf/util/parse-events.c >> +++ b/tools/perf/util/parse-events.c >> @@ -1231,12 +1231,17 @@ int parse_events_term__clone(struct >> parse_events_term **new, >> term->val.str, term->val.num); >> } >> >> -void parse_events__free_terms(struct list_head *terms) >> +void parse_events__free_terms_only(struct list_head *terms) >> { >> struct parse_events_term *term, *h; >> >> list_for_each_entry_safe(term, h, terms, list) >> free(term); >> +} >> + >> +void parse_events__free_terms(struct list_head *terms) >> +{ >> + parse_events__free_terms_only(terms); >> >> free(terms); >> } > > I still don't understand the reasoning for an _only function. There is only > 1 place that mallocs the list_head and that 1 user should free its own > memory. All of the other users pass a stack variable.
No. See parse-events.y
The list head is defined as a pointer in the YYTYPE stack element:
%union { char *str; u64 num; struct list_head *head; struct parse_events_term *term; }
It is malloc'ed when terms are created:
event_term { struct list_head *head = malloc(sizeof(*head)); struct parse_events_term *term = $1;
ABORT_ON(!head); INIT_LIST_HEAD(head); list_add_tail(&term->list, head); $$ = head; }
| |