On Tue, Oct 22, 2019 at 05:53:29PM -0700, Ian Rogers wrote:
Parse event error handling may overwrite one error string with another
creating memory leaks and masking errors. Introduce a helper routine
that appends error messages and avoids the memory leak.
good idea, it became little messy with time ;-)
some comments below
thanks,
jirka
quoted hunk ↗ jump to hunk
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/util/parse-events.c | 102 ++++++++++++++++++++++-----------
tools/perf/util/parse-events.h | 2 +
tools/perf/util/pmu.c | 36 ++++++------
3 files changed, 89 insertions(+), 51 deletions(-)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index db882f630f7e..4d42344698b8 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -182,6 +182,34 @@ static int tp_event_has_id(const char *dir_path, struct dirent *evt_dir)
#define MAX_EVENT_LENGTH 512
+void parse_events__append_error(struct parse_events_error *err, int idx,
+ char *str, char *help)
+{
+ char *new_str = NULL;
+
+ WARN(!str, "WARNING: failed to provide error string");
should we also bail out if str is NULL?
quoted hunk ↗ jump to hunk
+ if (err->str) {
+ int ret;
+
+ if (err->help)
+ ret = asprintf(&new_str,
+ "%s (previous error: %s(help: %s))",
+ str, err->str, err->help);
+ else
please use {} for multiline condition legs
quoted hunk ↗ jump to hunk
+ ret = asprintf(&new_str,
+ "%s (previous error: %s)",
+ str, err->str);
does this actualy happen? could you please provide output
of this in the changelog?
quoted hunk ↗ jump to hunk
+ if (ret < 0)
+ new_str = NULL;
+ else
+ zfree(&str);
+ }
+ err->idx = idx;
+ free(err->str);
+ err->str = new_str ?: str;
+ free(err->help);
+ err->help = help;
+}
SNIP