On 2/8/24 1:09 AM, Yafang Shao wrote:
On success, bpf_iter_<type>_new() return 0. On failure, it returns ERR.
We'd better check the return value of it.
Not sure whether this patch is necessary or not.
I checked:
bpf_iter_num_{new,next}
bpf_iter_task_vma_{new,next}
bpf_iter_css_task_{new,next}
It looks like the convention is for *_next() return NULL or not
instead of relying on return value of _new() to decide whether to
proceed or not. Maybe Andrii can clarify.
Signed-off-by: Yafang Shao <laoar.shao@xxxxxxxxx>
---
tools/lib/bpf/bpf_helpers.h | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 79eaa581be98..2cd2428e3bc6 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -133,6 +133,15 @@
# define __bpf_unreachable() __builtin_trap()
#endif
+#ifndef __must_check
+#define __must_check __attribute__((warn_unused_result))
+#endif
+
+static inline void * __must_check ERR_PTR(long error)
+{
+ return (void *) error;
+}
+
/*
* Helper function to perform a tail call with a constant/immediate map slot.
*/
@@ -340,14 +349,13 @@ extern void bpf_iter_num_destroy(struct bpf_iter_num *it) __weak __ksym;
/* initialize and define destructor */ \
struct bpf_iter_##type ___it __attribute__((aligned(8), /* enforce, just in case */, \
cleanup(bpf_iter_##type##_destroy))), \
- /* ___p pointer is just to call bpf_iter_##type##_new() *once* to init ___it */ \
*___p __attribute__((unused)) = ( \
- bpf_iter_##type##_new(&___it, ##args), \
/* this is a workaround for Clang bug: it currently doesn't emit BTF */ \
/* for bpf_iter_##type##_destroy() when used from cleanup() attribute */ \
- (void)bpf_iter_##type##_destroy, (void *)0); \
+ (void)bpf_iter_##type##_destroy, \
+ ERR_PTR(bpf_iter_##type##_new(&___it, ##args))); \
/* iteration and termination check */ \
- (((cur) = bpf_iter_##type##_next(&___it))); \
+ ((!___p) && ((cur) = bpf_iter_##type##_next(&___it))); \
)
#endif /* bpf_for_each */
