BPF List
 help / color / mirror / Atom feed
* [PATCH 1/2] perf tools: Fix possible compiler warnings in hashmap
@ 2024-10-09 20:20 Namhyung Kim
  2024-10-11  1:48 ` Andrii Nakryiko
  2024-10-16 17:12 ` Namhyung Kim
  0 siblings, 2 replies; 5+ messages in thread
From: Namhyung Kim @ 2024-10-09 20:20 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ian Rogers, Kan Liang
  Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
	linux-perf-users, bpf

The hashmap__for_each_entry[_safe] is accessing 'map' as if it's a
pointer.  But it does without parentheses so passing a static hash map
with an ampersand (like &slab_hash below) caused compiler warnings due
to unmatched types.

  In file included from util/bpf_lock_contention.c:5:
  util/bpf_lock_contention.c: In function ‘exit_slab_cache_iter’:
  linux/tools/perf/util/hashmap.h:169:32: error: invalid type argument of ‘->’ (have ‘struct hashmap’)
    169 |         for (bkt = 0; bkt < map->cap; bkt++)                                \
        |                                ^~
  util/bpf_lock_contention.c:105:9: note: in expansion of macro ‘hashmap__for_each_entry’
    105 |         hashmap__for_each_entry(&slab_hash, cur, bkt)
        |         ^~~~~~~~~~~~~~~~~~~~~~~
  /home/namhyung/project/linux/tools/perf/util/hashmap.h:170:31: error: invalid type argument of ‘->’ (have ‘struct hashmap’)
    170 |                 for (cur = map->buckets[bkt]; cur; cur = cur->next)
        |                               ^~
  util/bpf_lock_contention.c:105:9: note: in expansion of macro ‘hashmap__for_each_entry’
    105 |         hashmap__for_each_entry(&slab_hash, cur, bkt)
        |         ^~~~~~~~~~~~~~~~~~~~~~~

Cc: bpf@vger.kernel.org
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
I've discovered this while prototyping the slab symbolization for perf
lock contention.  So this code is not available yet but I'd like to fix
the problem first.

Also noticed bpf has the same code and the same problem.  I'll send a
separate patch for them.

 tools/perf/util/hashmap.h | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/hashmap.h b/tools/perf/util/hashmap.h
index c12f8320e6682d50..0c4f155e8eb745d9 100644
--- a/tools/perf/util/hashmap.h
+++ b/tools/perf/util/hashmap.h
@@ -166,8 +166,8 @@ bool hashmap_find(const struct hashmap *map, long key, long *value);
  * @bkt: integer used as a bucket loop cursor
  */
 #define hashmap__for_each_entry(map, cur, bkt)				    \
-	for (bkt = 0; bkt < map->cap; bkt++)				    \
-		for (cur = map->buckets[bkt]; cur; cur = cur->next)
+	for (bkt = 0; bkt < (map)->cap; bkt++)				    \
+		for (cur = (map)->buckets[bkt]; cur; cur = cur->next)
 
 /*
  * hashmap__for_each_entry_safe - iterate over all entries in hashmap, safe
@@ -178,8 +178,8 @@ bool hashmap_find(const struct hashmap *map, long key, long *value);
  * @bkt: integer used as a bucket loop cursor
  */
 #define hashmap__for_each_entry_safe(map, cur, tmp, bkt)		    \
-	for (bkt = 0; bkt < map->cap; bkt++)				    \
-		for (cur = map->buckets[bkt];				    \
+	for (bkt = 0; bkt < (map)->cap; bkt++)				    \
+		for (cur = (map)->buckets[bkt];				    \
 		     cur && ({tmp = cur->next; true; });		    \
 		     cur = tmp)
 
@@ -190,19 +190,19 @@ bool hashmap_find(const struct hashmap *map, long key, long *value);
  * @key: key to iterate entries for
  */
 #define hashmap__for_each_key_entry(map, cur, _key)			    \
-	for (cur = map->buckets						    \
-		     ? map->buckets[hash_bits(map->hash_fn((_key), map->ctx), map->cap_bits)] \
+	for (cur = (map)->buckets					    \
+		     ? (map)->buckets[hash_bits((map)->hash_fn((_key), (map)->ctx), (map)->cap_bits)] \
 		     : NULL;						    \
 	     cur;							    \
 	     cur = cur->next)						    \
-		if (map->equal_fn(cur->key, (_key), map->ctx))
+		if ((map)->equal_fn(cur->key, (_key), (map)->ctx))
 
 #define hashmap__for_each_key_entry_safe(map, cur, tmp, _key)		    \
-	for (cur = map->buckets						    \
-		     ? map->buckets[hash_bits(map->hash_fn((_key), map->ctx), map->cap_bits)] \
+	for (cur = (map)->buckets					    \
+		     ? (map)->buckets[hash_bits((map)->hash_fn((_key), (map)->ctx), (map)->cap_bits)] \
 		     : NULL;						    \
 	     cur && ({ tmp = cur->next; true; });			    \
 	     cur = tmp)							    \
-		if (map->equal_fn(cur->key, (_key), map->ctx))
+		if ((map)->equal_fn(cur->key, (_key), (map)->ctx))
 
 #endif /* __LIBBPF_HASHMAP_H */
-- 
2.47.0.rc0.187.ge670bccf7e-goog


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-10-16 17:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 20:20 [PATCH 1/2] perf tools: Fix possible compiler warnings in hashmap Namhyung Kim
2024-10-11  1:48 ` Andrii Nakryiko
2024-10-11 16:44   ` Namhyung Kim
2024-10-11 18:00     ` Andrii Nakryiko
2024-10-16 17:12 ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox