The hashmap implementation in tools/perf/util was using ERR_PTR to return errors from perf_hashmap__new. This is non-standard for most of the perf codebase which prefers NULL on error and setting errno. As such it was a frequent source of bugs: commit d05073adda0f ("perf trace: Avoid an ERR_PTR in syscall_stats") commit 96f202eab813 ("perf trace: Fix IS_ERR() vs NULL check bug") commit 9f3c16a430e8 ("perf expr: Fix return value of ids__new()") commit 4d4d00dd321f ("perf tools: Update copy of libbpf's hashmap.c") Remove the dependency on linux/err.h in hashmap.c. Update perf_hashmap__new to set errno = ENOMEM and return NULL on failure. Update perf_hashmap__free to check for NULL instead of using IS_ERR_OR_NULL. Update all callers of perf_hashmap__new to check for NULL. In places where IS_ERR_OR_NULL was used on hashmap pointers, switch to regular NULL checks. Signed-off-by: Ian Rogers --- tools/perf/builtin-trace.c | 2 +- tools/perf/ui/browsers/annotate.c | 4 ++-- tools/perf/util/expr.c | 4 ++-- tools/perf/util/fncache.c | 10 +++++++--- tools/perf/util/hashmap.c | 9 +++++---- tools/perf/util/pmu.c | 6 ++++++ tools/perf/util/s390-sample-raw.c | 2 +- tools/perf/util/stat.c | 2 +- 8 files changed, 25 insertions(+), 14 deletions(-) diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index 88b2fac16457..7b186c32cb0d 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -1566,7 +1566,7 @@ static struct perf_hashmap *alloc_syscall_stats(void) { struct perf_hashmap *result = perf_hashmap__new(syscall_id_hash, syscall_id_equal, NULL); - return IS_ERR(result) ? NULL : result; + return result; } static void delete_syscall_stats(struct perf_hashmap *syscall_stats) diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index c61415295dda..5c0656421084 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -143,7 +143,7 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int if (!browser->navkeypressed) ops.width += 1; - if (!IS_ERR_OR_NULL(ab->type_hash)) + if (ab->type_hash) apd.type_hash = ab->type_hash; annotation_line__write(al, notes, &ops, &apd); @@ -1248,7 +1248,7 @@ int __hist_entry__tui_annotate(struct hist_entry *he, struct map_symbol *ms, debuginfo__delete(browser.dbg); - if (!IS_ERR_OR_NULL(browser.type_hash)) { + if (browser.type_hash) { struct perf_hashmap_entry *cur; size_t bkt; diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c index dc10c335e378..f96812bae318 100644 --- a/tools/perf/util/expr.c +++ b/tools/perf/util/expr.c @@ -68,7 +68,7 @@ struct perf_hashmap *ids__new(void) struct perf_hashmap *hash; hash = perf_hashmap__new(key_hash, key_equal, NULL); - if (IS_ERR(hash)) + if (!hash) return NULL; return hash; } @@ -296,7 +296,7 @@ struct expr_parse_ctx *expr__ctx_new(void) return NULL; ctx->ids = perf_hashmap__new(key_hash, key_equal, NULL); - if (IS_ERR(ctx->ids)) { + if (!ctx->ids) { free(ctx); return NULL; } diff --git a/tools/perf/util/fncache.c b/tools/perf/util/fncache.c index 9c49a914e784..0970b0bbd1d5 100644 --- a/tools/perf/util/fncache.c +++ b/tools/perf/util/fncache.c @@ -36,9 +36,10 @@ static struct perf_hashmap *fncache__get(void) static bool lookup_fncache(const char *name, bool *res) { + struct perf_hashmap *map = fncache__get(); long val; - if (!perf_hashmap__find(fncache__get(), name, &val)) + if (!map || !perf_hashmap__find(map, name, &val)) return false; *res = (val != 0); @@ -47,11 +48,14 @@ static bool lookup_fncache(const char *name, bool *res) static void update_fncache(const char *name, bool res) { + struct perf_hashmap *map = fncache__get(); char *old_key = NULL, *key = strdup(name); - if (key) { - perf_hashmap__set(fncache__get(), key, res, &old_key, /*old_value*/NULL); + if (map && key) { + perf_hashmap__set(map, key, res, &old_key, /*old_value*/NULL); free(old_key); + } else { + free(key); } } diff --git a/tools/perf/util/hashmap.c b/tools/perf/util/hashmap.c index d90ef4ed384d..147e011a547f 100644 --- a/tools/perf/util/hashmap.c +++ b/tools/perf/util/hashmap.c @@ -10,7 +10,6 @@ #include #include #include -#include #include "hashmap.h" /* make sure libbpf doesn't use kernel-only integer typedefs */ @@ -55,8 +54,10 @@ struct perf_hashmap *perf_hashmap__new(perf_hashmap_hash_fn hash_fn, { struct perf_hashmap *map = malloc(sizeof(struct perf_hashmap)); - if (!map) - return ERR_PTR(-ENOMEM); + if (!map) { + errno = ENOMEM; + return NULL; + } perf_hashmap__init(map, hash_fn, equal_fn, ctx); return map; } @@ -76,7 +77,7 @@ void perf_hashmap__clear(struct perf_hashmap *map) void perf_hashmap__free(struct perf_hashmap *map) { - if (IS_ERR_OR_NULL(map)) + if (!map) return; perf_hashmap__clear(map); diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c index ace354b23501..01a7c0098797 100644 --- a/tools/perf/util/pmu.c +++ b/tools/perf/util/pmu.c @@ -1305,6 +1305,12 @@ struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pm INIT_LIST_HEAD(&pmu->format); pmu->aliases = perf_hashmap__new(aliases__hash, aliases__equal, /*ctx=*/ NULL); + if (!pmu->aliases) { + perf_cpu_map__put(pmu->cpus); + free((char *)pmu->name); + free(pmu); + return NULL; + } INIT_LIST_HEAD(&pmu->caps); list_add_tail(&pmu->list, core_pmus); return pmu; diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c index 80a5535089e0..52ab84e53173 100644 --- a/tools/perf/util/s390-sample-raw.c +++ b/tools/perf/util/s390-sample-raw.c @@ -201,7 +201,7 @@ static char *get_counter_name(int set, int nr, struct perf_pmu *pmu) get_counter_name_perf_hashmap_equal_fn, /*ctx=*/NULL); - if (!IS_ERR(tmp)) { + if (tmp) { cache = tmp; cache_pmu = pmu; } diff --git a/tools/perf/util/stat.c b/tools/perf/util/stat.c index 81b266c5d4f5..1359a1f81562 100644 --- a/tools/perf/util/stat.c +++ b/tools/perf/util/stat.c @@ -320,7 +320,7 @@ static int check_per_pkg(struct evsel *counter, struct perf_counts_values *vals, if (!mask) { mask = perf_hashmap__new(pkg_id_hash, pkg_id_equal, NULL); - if (IS_ERR(mask)) + if (!mask) return -ENOMEM; counter->per_pkg_mask = mask; -- 2.53.0.959.g497ff81fa9-goog