[PATCH v1 07/26] perf hashmap: Remove errptr usage from hashmap

[Ian Rogers <irogers@google.com>]

The hashmap implementation in tools/perf/util was using ERR_PTR to
return errors from 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
__hashmap__new() to set errno = ENOMEM and return NULL on failure, and
update __hashmap__free() to check for NULL instead of using
IS_ERR_OR_NULL().

Update all callers of hashmap__new() to check for NULL (including
missing failure checks in fncache and
perf_pmu__create_placeholder_core_pmu()), and switch IS_ERR_OR_NULL()
checks on hashmap pointers to regular NULL checks.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-ftrace.c       |  9 +--------
 tools/perf/builtin-trace.c        |  4 +---
 tools/perf/ui/browsers/annotate.c |  4 ++--
 tools/perf/util/annotate.c        |  4 +---
 tools/perf/util/expr.c            |  9 ++-------
 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 +-
 10 files changed, 27 insertions(+), 32 deletions(-)

diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c
index 7cdc9ae2fbbb..ac0107f5b891 100644
--- a/tools/perf/builtin-ftrace.c
+++ b/tools/perf/builtin-ftrace.c
@@ -1229,14 +1229,7 @@ static int prepare_func_profile(struct perf_ftrace *ftrace)
 	ftrace->graph_verbose = 0;
 
 	ftrace->profile_hash = hashmap__new(profile_hash, profile_equal, NULL);
-	if (IS_ERR(ftrace->profile_hash)) {
-		int err = PTR_ERR(ftrace->profile_hash);
-
-		ftrace->profile_hash = NULL;
-		return err;
-	}
-
-	return 0;
+	return ftrace->profile_hash ? 0 : -ENOMEM;
 }
 
 /* This is saved in a hashmap keyed by the function name */
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 4094141461f9..128efe1be8c3 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -1673,9 +1673,7 @@ static bool syscall_id_equal(long key1, long key2, void *ctx __maybe_unused)
 
 static struct hashmap *alloc_syscall_stats(void)
 {
-	struct hashmap *result = hashmap__new(syscall_id_hash, syscall_id_equal, NULL);
-
-	return IS_ERR(result) ? NULL : result;
+	return hashmap__new(syscall_id_hash, syscall_id_equal, NULL);
 }
 
 static void delete_syscall_stats(struct hashmap *syscall_stats)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index cf36b5347f72..8b0c933caf2e 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -144,7 +144,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);
@@ -1264,7 +1264,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 hashmap_entry *cur;
 		size_t bkt;
 
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 9af1d6b72da9..d11add6424dc 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -137,10 +137,8 @@ static int annotated_source__alloc_histograms(struct annotated_source *src,
 		return -1;
 
 	src->samples = hashmap__new(sym_hist_hash, sym_hist_equal, NULL);
-	if (IS_ERR(src->samples)) {
+	if (!src->samples)
 		zfree(&src->histograms);
-		src->samples = NULL;
-	}
 
 	return src->histograms ? 0 : -1;
 }
diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
index f1164c4666b1..ac0243576def 100644
--- a/tools/perf/util/expr.c
+++ b/tools/perf/util/expr.c
@@ -66,12 +66,7 @@ static bool key_equal(long key1, long key2, void *ctx __maybe_unused)
 
 struct hashmap *ids__new(void)
 {
-	struct hashmap *hash;
-
-	hash = hashmap__new(key_hash, key_equal, NULL);
-	if (IS_ERR(hash))
-		return NULL;
-	return hash;
+	return hashmap__new(key_hash, key_equal, NULL);
 }
 
 void ids__free(struct hashmap *ids)
@@ -306,7 +301,7 @@ struct expr_parse_ctx *expr__ctx_new(void)
 		return NULL;
 
 	ctx->ids = 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 bf9559c55c63..04ea150bc7cd 100644
--- a/tools/perf/util/fncache.c
+++ b/tools/perf/util/fncache.c
@@ -36,9 +36,10 @@ static struct hashmap *fncache__get(void)
 
 static bool lookup_fncache(const char *name, bool *res)
 {
+	struct hashmap *map = fncache__get();
 	long val;
 
-	if (!hashmap__find(fncache__get(), name, &val))
+	if (!map || !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 hashmap *map = fncache__get();
 	char *old_key = NULL, *key = strdup(name);
 
-	if (key) {
-		hashmap__set(fncache__get(), key, res, &old_key, /*old_value*/NULL);
+	if (map && key) {
+		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 7fd87c7d72be..f2d8beeac27a 100644
--- a/tools/perf/util/hashmap.c
+++ b/tools/perf/util/hashmap.c
@@ -10,7 +10,6 @@
 #include <stdio.h>
 #include <string.h>
 #include <errno.h>
-#include <linux/err.h>
 #include "hashmap.h"
 
 /* make sure libbpf doesn't use kernel-only integer typedefs */
@@ -55,8 +54,10 @@ struct hashmap *__hashmap__new(hashmap_hash_fn hash_fn,
 {
 	struct hashmap *map = malloc(sizeof(struct hashmap));
 
-	if (!map)
-		return ERR_PTR(-ENOMEM);
+	if (!map) {
+		errno = ENOMEM;
+		return NULL;
+	}
 	__hashmap__init(map, hash_fn, equal_fn, ctx);
 	return map;
 }
@@ -76,7 +77,7 @@ void __hashmap__clear(struct hashmap *map)
 
 void __hashmap__free(struct hashmap *map)
 {
-	if (IS_ERR_OR_NULL(map))
+	if (!map)
 		return;
 
 	__hashmap__clear(map);
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 01753d5c69d1..a63fa0d9cfa5 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1314,6 +1314,12 @@ struct perf_pmu *perf_pmu__create_placeholder_core_pmu(struct list_head *core_pm
 
 	INIT_LIST_HEAD(&pmu->format);
 	pmu->aliases = 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 aaf8838b2cb2..e0fac2aa15ed 100644
--- a/tools/perf/util/s390-sample-raw.c
+++ b/tools/perf/util/s390-sample-raw.c
@@ -202,7 +202,7 @@ static char *get_counter_name(int set, int nr, struct perf_pmu *pmu)
 						   get_counter_name_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 e7013449271d..097985da3dbb 100644
--- a/tools/perf/util/stat.c
+++ b/tools/perf/util/stat.c
@@ -326,7 +326,7 @@ static int check_per_pkg(struct evsel *counter, struct perf_counts_values *vals,
 
 	if (!mask) {
 		mask = hashmap__new(pkg_id_hash, pkg_id_equal, NULL);
-		if (IS_ERR(mask))
+		if (!mask)
 			return -ENOMEM;
 
 		counter->per_pkg_mask = mask;
-- 
2.56.1.703.gf848ed807c-goog