Thread (9 messages) flat view 9 messages, 2 authors, 2d ago

Re: [PATCH v5 4/5] rv/reactors: add KUnit tests for reactor registration and dispatch

From: sashiko-bot@kernel.org
Date: 2026-09-06 17:23:07

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Typos in commit message and comments, and non-standard use of _Static_assert.
--

commit a14edcfd6c547b97a3d6e82ee3b87a759234cdc5
Author: Wen Yang [off-list ref]

rv/reactors: add KUnit tests for reactor registration and dispatch

This commit adds KUnit tests to cover the reactor register and unregister
lifecycle, including duplicate and name-length rejection. It also tests
the rv_react() dispatch logic.
The dispatch tests rely o reacting_on being enabled, since rv_react()
returns early when it if off.
[Severity: Low]
There appear to be a few typos in the commit message: "rely o" instead of
"rely on", and "it if off" instead of "it is off".

[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/rv/rv_reactors_kunit.c b/kernel/trace/rv/rv_reactors_kunit.c
new file mode 100644
index 0000000000000..a408edcdde04f
--- /dev/null
+++ b/kernel/trace/rv/rv_reactors_kunit.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * KUnit tests for RV reactor registration and dispatch.
+ *
+ * The dispatch tests rely on reacting_on beinng enabled, since rv_react()
+ * returns early when it is off. It is on by default when the suites run
+ * built-in; as a module, re-enable it if disabled via
+ * /sys/kerne/tracing/rv/reacting_on.
+ */
[Severity: Low]
There are a couple of typos in this file header comment: "beinng" instead of
"being", and "/sys/kerne/" instead of "/sys/kernel/".

[ ... ]
+static const char long_reactor_name[] = "kunit_reactor_name_too_long_xxx_";
+_Static_assert(sizeof(long_reactor_name) - 1 >= MAX_RV_REACTOR_NAME_SIZE,
+	       "long_reactor_name must be at least MAX_RV_REACTOR_NAME_SIZE chars");
+
+static void test_name_too_long(struct kunit *test)
[Severity: Low]
Does this code use the C11 keyword _Static_assert directly rather than the
kernel's standard static_assert macro? The kernel typically wraps this in
include/linux/build_bug.h as static_assert(), which is the standard
convention for compile-time assertions across the tree.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1788705281.git.wen.yang@linux.dev?part=4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help