On Thu, Oct 24, 2019 at 12:01:56PM -0700, Ian Rogers wrote:
> Make it easier to release memory associated with parse event terms by
> duplicating the string for the config name and ensuring the val string
> is a duplicate.
>
> Currently the parser may memory leak terms and this is addressed in a
> later patch.
please move that patch before this one
thanks,
jirka
>
> Signed-off-by: Ian Rogers <irogers@xxxxxxxxxx>
> ---
> tools/perf/util/parse-events.c | 51 ++++++++++++++++++++++++++++------
> tools/perf/util/parse-events.y | 4 ++-
> 2 files changed, 45 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index f0d50f079d2f..dc5862a663b5 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -1430,7 +1430,6 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
> int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> char *str, struct list_head **listp)
> {
> - struct list_head *head;
> struct parse_events_term *term;
> struct list_head *list;
> struct perf_pmu *pmu = NULL;
> @@ -1447,19 +1446,30 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
>
> list_for_each_entry(alias, &pmu->aliases, list) {
> if (!strcasecmp(alias->name, str)) {
> + struct list_head *head;
> + char *config;
> +
> head = malloc(sizeof(struct list_head));
> if (!head)
> return -1;
> INIT_LIST_HEAD(head);
> - if (parse_events_term__num(&term, PARSE_EVENTS__TERM_TYPE_USER,
> - str, 1, false, &str, NULL) < 0)
> + config = strdup(str);
> + if (!config)
> + return -1;
> + if (parse_events_term__num(&term,
> + PARSE_EVENTS__TERM_TYPE_USER,
> + config, 1, false, &config,
> + NULL) < 0) {
> + free(list);
> + free(config);
> return -1;
> + }
> list_add_tail(&term->list, head);
>
> if (!parse_events_add_pmu(parse_state, list,
> pmu->name, head,
> true, true)) {
> - pr_debug("%s -> %s/%s/\n", str,
> + pr_debug("%s -> %s/%s/\n", config,
> pmu->name, alias->str);
> ok++;
> }
> @@ -1468,8 +1478,10 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
> }
> }
> }
> - if (!ok)
> + if (!ok) {
> + free(list);
> return -1;
> + }
> *listp = list;
> return 0;
> }
> @@ -2764,30 +2776,51 @@ int parse_events_term__sym_hw(struct parse_events_term **term,
> char *config, unsigned idx)
> {
> struct event_symbol *sym;
> + char *str;
> struct parse_events_term temp = {
> .type_val = PARSE_EVENTS__TERM_TYPE_STR,
> .type_term = PARSE_EVENTS__TERM_TYPE_USER,
> - .config = config ?: (char *) "event",
> + .config = config,
> };
>
> + if (!temp.config) {
> + temp.config = strdup("event");
> + if (!temp.config)
> + return -ENOMEM;
> + }
> BUG_ON(idx >= PERF_COUNT_HW_MAX);
> sym = &event_symbols_hw[idx];
>
> - return new_term(term, &temp, (char *) sym->symbol, 0);
> + str = strdup(sym->symbol);
> + if (!str)
> + return -ENOMEM;
> + return new_term(term, &temp, str, 0);
> }
>
> int parse_events_term__clone(struct parse_events_term **new,
> struct parse_events_term *term)
> {
> + char *str;
> struct parse_events_term temp = {
> .type_val = term->type_val,
> .type_term = term->type_term,
> - .config = term->config,
> + .config = NULL,
> .err_term = term->err_term,
> .err_val = term->err_val,
> };
>
> - return new_term(new, &temp, term->val.str, term->val.num);
> + if (term->config) {
> + temp.config = strdup(term->config);
> + if (!temp.config)
> + return -ENOMEM;
> + }
> + if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM)
> + return new_term(new, &temp, NULL, term->val.num);
> +
> + str = strdup(term->val.str);
> + if (!str)
> + return -ENOMEM;
> + return new_term(new, &temp, str, 0);
> }
>
> int parse_events_copy_term_list(struct list_head *old,
> diff --git a/tools/perf/util/parse-events.y b/tools/perf/util/parse-events.y
> index 48126ae4cd13..27d6b187c9b1 100644
> --- a/tools/perf/util/parse-events.y
> +++ b/tools/perf/util/parse-events.y
> @@ -644,9 +644,11 @@ PE_NAME array '=' PE_VALUE
> PE_DRV_CFG_TERM
> {
> struct parse_events_term *term;
> + char *config = strdup($1);
>
> + ABORT_ON(!config);
> ABORT_ON(parse_events_term__str(&term, PARSE_EVENTS__TERM_TYPE_DRV_CFG,
> - $1, $1, &@1, NULL));
> + config, $1, &@1, NULL));
> $$ = term;
> }
>
> --
> 2.23.0.866.gb869b98d4c-goog
>
