On Fri, Oct 25, 2019 at 1:27 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote: > > On Thu, Oct 24, 2019 at 12:01:59PM -0700, Ian Rogers wrote: > > If parsing fails then destructors are ran to clean the up the stack. > > Rename the head union member to make the term and evlist use cases more > > distinct, this simplifies matching the correct destructor. > > nice did not know about this.. looks like it's been in bison for some time, right? Looks like it wasn't in Bison 1 but in Bison 2, we're at Bison 3 and Bison 2 is > 14 years old: https://web.archive.org/web/20050924004158/http://www.gnu.org/software/bison/manual/html_mono/bison.html#Destructor-Decl > > > > Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx> > > --- > > tools/perf/util/parse-events.y | 69 +++++++++++++++++++++++----------- > > 1 file changed, 48 insertions(+), 21 deletions(-) > > > > diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y > > index 545ab7cefc20..4725b14b9db4 100644 > > --- a/tools/perf/util/parse-events.y > > +++ b/tools/perf/util/parse-events.y > > @@ -12,6 +12,7 @@ > > #include <stdio.h> > > #include <linux/compiler.h> > > #include <linux/types.h> > > +#include <linux/zalloc.h> > > #include "pmu.h" > > #include "evsel.h" > > #include "parse-events.h" > > @@ -37,6 +38,25 @@ static struct list_head* alloc_list() > > return list; > > } > > > > +static void free_list_evsel(struct list_head* list_evsel) > > +{ > > + struct perf_evsel *pos, *tmp; > > + > > + list_for_each_entry_safe(pos, tmp, list_evsel, node) { > > + list_del_init(&pos->node); > > + perf_evsel__delete(pos); > > + } > > + free(list_evsel); > > I think you need to iterate 'struct evsel' in here, not 'struct perf_evsel' > > should be: > > struct evsel *evsel, *tmp; > > list_for_each_entry_safe(evsel, tmp, list_evsel, core.node) { > list_del_init(&evsel->core.node); > evsel__delete(evsel); > } Thanks, I'll address this. Ian > thanks, > jirka > > > +} > > + > > +static void free_term(struct parse_events_term *term) > > +{ > > + if (term->type_val == PARSE_EVENTS__TERM_TYPE_STR) > > + free(term->val.str); > > + zfree(&term->array.ranges); > > + free(term); > > +} > > + > > static void inc_group_count(struct list_head *list, > > struct parse_events_state *parse_state) > > { > > @@ -66,6 +86,7 @@ static void inc_group_count(struct list_head *list, > > %type <num> PE_VALUE_SYM_TOOL > > %type <num> PE_RAW > > %type <num> PE_TERM > > +%type <num> value_sym > > %type <str> PE_NAME > > %type <str> PE_BPF_OBJECT > > %type <str> PE_BPF_SOURCE > > @@ -76,37 +97,43 @@ static void inc_group_count(struct list_head *list, > > %type <str> PE_EVENT_NAME > > %type <str> PE_PMU_EVENT_PRE PE_PMU_EVENT_SUF PE_KERNEL_PMU_EVENT > > %type <str> PE_DRV_CFG_TERM > > -%type <num> value_sym > > -%type <head> event_config > > -%type <head> opt_event_config > > -%type <head> opt_pmu_config > > +%destructor { free ($$); } <str> > > %type <term> event_term > > -%type <head> event_pmu > > -%type <head> event_legacy_symbol > > -%type <head> event_legacy_cache > > -%type <head> event_legacy_mem > > -%type <head> event_legacy_tracepoint > > +%destructor { free_term ($$); } <term> > > +%type <list_terms> event_config > > +%type <list_terms> opt_event_config > > +%type <list_terms> opt_pmu_config > > +%destructor { parse_events_terms__delete ($$); } <list_terms> > > +%type <list_evsel> event_pmu > > +%type <list_evsel> event_legacy_symbol > > +%type <list_evsel> event_legacy_cache > > +%type <list_evsel> event_legacy_mem > > +%type <list_evsel> event_legacy_tracepoint > > +%type <list_evsel> event_legacy_numeric > > +%type <list_evsel> event_legacy_raw > > +%type <list_evsel> event_bpf_file > > +%type <list_evsel> event_def > > +%type <list_evsel> event_mod > > +%type <list_evsel> event_name > > +%type <list_evsel> event > > +%type <list_evsel> events > > +%type <list_evsel> group_def > > +%type <list_evsel> group > > +%type <list_evsel> groups > > +%destructor { free_list_evsel ($$); } <list_evsel> > > %type <tracepoint_name> tracepoint_name > > -%type <head> event_legacy_numeric > > -%type <head> event_legacy_raw > > -%type <head> event_bpf_file > > -%type <head> event_def > > -%type <head> event_mod > > -%type <head> event_name > > -%type <head> event > > -%type <head> events > > -%type <head> group_def > > -%type <head> group > > -%type <head> groups > > +%destructor { free ($$.sys); free ($$.event); } <tracepoint_name> > > %type <array> array > > %type <array> array_term > > %type <array> array_terms > > +%destructor { free ($$.ranges); } <array> > > > > %union > > { > > char *str; > > u64 num; > > - struct list_head *head; > > + struct list_head *list_evsel; > > + struct list_head *list_terms; > > struct parse_events_term *term; > > struct tracepoint_name { > > char *sys; > > -- > > 2.23.0.866.gb869b98d4c-goog > > >