Em Thu, Jun 11, 2020 at 09:42:34AM +0800, Chen Wandun escreveu: > Fix potential memory leak in function parse_events_term__sym_hw() > and parse_events_term__clone(). > > 1. Free memory when errors occur. > 2. Function new_term may return error, so it is need to free memory > when the return value is negative.
Try to fix one thing per patch, i.e. first the most obvious one, then the other that requires going thru the new_term() logic, i.e. first this, which is super easy to review: diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c index c4906a6a9f1a..3ada3874a90a 100644 --- a/tools/perf/util/parse-events.c +++ b/tools/perf/util/parse-events.c @@ -2958,8 +2958,10 @@ int parse_events_term__sym_hw(struct parse_events_term **term, sym = &event_symbols_hw[idx]; str = strdup(sym->symbol); - if (!str) + if (!str) { + zfree(&temp.config); return -ENOMEM; + } return new_term(term, &temp, str, 0); } @@ -2984,8 +2986,10 @@ int parse_events_term__clone(struct parse_events_term **new, return new_term(new, &temp, NULL, term->val.num); str = strdup(term->val.str); - if (!str) + if (!str) { + zfree(&temp.config); return -ENOMEM; + } return new_term(new, &temp, str, 0); } Then you go to the one that requires the reviewer (now or in the future, trying to figure out why something broke) to look at the new_term() code, ok? - Arnaldo > Signed-off-by: Chen Wandun <chenwan...@huawei.com> > --- > tools/perf/util/parse-events.c | 40 +++++++++++++++++++++++++++++----- > 1 file changed, 34 insertions(+), 6 deletions(-) > > diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c > index 3decbb203846..3491c18edd71 100644 > --- a/tools/perf/util/parse-events.c > +++ b/tools/perf/util/parse-events.c > @@ -2947,6 +2947,7 @@ int parse_events_term__sym_hw(struct parse_events_term > **term, > .type_term = PARSE_EVENTS__TERM_TYPE_USER, > .config = config, > }; > + int ret; > > if (!temp.config) { > temp.config = strdup("event"); > @@ -2957,9 +2958,20 @@ int parse_events_term__sym_hw(struct parse_events_term > **term, > sym = &event_symbols_hw[idx]; > > str = strdup(sym->symbol); > - if (!str) > + if (!str) { > + if (!config) > + free(temp.config); > return -ENOMEM; > - return new_term(term, &temp, str, 0); > + } > + > + ret = new_term(term, &temp, str, 0); > + if (ret < 0) { > + free(str); > + if (!config) > + free(temp.config); > + } > + > + return ret; > } > > int parse_events_term__clone(struct parse_events_term **new, > @@ -2973,19 +2985,35 @@ int parse_events_term__clone(struct parse_events_term > **new, > .err_term = term->err_term, > .err_val = term->err_val, > }; > + int ret; > > 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); > + if (term->type_val == PARSE_EVENTS__TERM_TYPE_NUM) { > + ret = new_term(new, &temp, NULL, term->val.num); > + if (ret < 0 && term->config) > + free(temp.config); > + return ret; > + } > > str = strdup(term->val.str); > - if (!str) > + if (!str) { > + if (term->config) > + free(temp.config); > return -ENOMEM; > - return new_term(new, &temp, str, 0); > + } > + > + ret = new_term(new, &temp, str, 0); > + if (ret < 0) { > + free(str); > + if (term->config) > + free(temp.config); > + } > + > + return ret; > } > > void parse_events_term__delete(struct parse_events_term *term) > -- > 2.17.1 > -- - Arnaldo