Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Typo in the test cleanup trap prevents the script from properly detaching the reactor and unloading the module on test failure. --
commit fcfb27a313bc6b0b71755c80b19e1e2cbf231cac Author: Wen Yang <[email protected]> selftests/verification: Test loadable module-based reactor Add a selftest module that register an RV reactor and a test that exercises the module pinning: rmmod must fail while a monitor is attached to the reactor and succeed after it is detached. A trap unloads the module on failure so it does not break the next insmod. > diff --git > a/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc > b/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc > new file mode 100644 > index 0000000000000..ff16916615215 > --- /dev/null > +++ b/tools/testing/selftests/verification/test.d/rv_reactor_loadable.tc > @@ -0,0 +1,46 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0-or-later > +# description: Test a loadable module-based reactor > +# requires: available_reactors insmod:program rmmod:program > + > +MODULE_KO="$FTRACETEST_ROOT/test_modules/rv_test_reactor.ko" > +[ -f "$MODULE_KO" ] || exit_unsupported > +[ -f /proc/modules ] || exit_unsupported > +monitor=$(ls monitors | head -n 1) > +[ -n "$monitor" ] || exit_unsupported > + > +cleanup() { > + if grep -q '^test_reactors$' available_reactors; then [Severity: Medium] Is there a typo in this grep pattern? Should it be test_reactor instead of test_reactors? If the test fails and triggers the cleanup trap, this condition will evaluate to false, bypassing the cleanup logic. This leaves the test module loaded and the reactor attached, which pollutes the test environment for subsequent selftests. > + echo nop > "monitors/$monitor/reactors" || true > + rmmod rv_test_reactor || true > + fi > +} -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
