On Sat, 22 Aug 2026 09:36:38 +0000 [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] Unreachable error handling logic due to early exit under 'set -e'. > -- > > --- Patch [7]: [PATCH v13 07/12] selftests: tracing: Add a basic testcase for > wprobe --- > commit 6d72a05750c5dfc08de6b03a6fc65e5144b20b9f > Author: Masami Hiramatsu (Google) <[email protected]> > > selftests: tracing: Add a basic testcase for wprobe > > Add an add_remove_wprobe.tc testcase for testing wprobe events that > tests adding and removing operations. > > > diff --git > > a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc > > b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc > > new file mode 100644 > > --- /dev/null > > +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_wprobe.tc > > @@ -0,0 +1,63 @@ > > +#!/bin/sh > > +# SPDX-License-Identifier: GPL-2.0 > > +# description: Generic dynamic event - add/remove wprobe events > > +# requires: dynamic_events "w[:[<group>/][<event>]] > > [r|w|rw]@<addr>[:<len>]":README > > + > > +echo 0 > events/enable > > +echo > dynamic_events > > + > > +# Use jiffies as a variable that is frequently written to. > > +TARGET=jiffies > > + > > +echo "w:my_wprobe w@$TARGET" >> dynamic_events > > + > > +if ! grep -q my_wprobe dynamic_events; then > > + echo "Failed to create wprobe event" > > + exit_fail > > +fi > > [Severity: Medium] > Since the ftracetest framework runs these scripts with set -e (errexit) > enabled, will this script exit immediately if the kernel rejects the event > creation and the echo command fails? > > If so, the subsequent diagnostic grep check and exit_fail call wouldn't be > reached, leading to a silent failure without the intended error message.
This is expected, the later grep checks whether if creation command succeeded but the event is not shown in the dynamic_events. Different check. > > + > > +if [ ! -d events/wprobes/my_wprobe ]; then > > + echo "Failed to create wprobe event directory" > > + exit_fail > > +fi > > + > > +echo 1 > events/wprobes/my_wprobe/enable > > + > > +# Check if the event is enabled > > +if ! grep -q 1 events/wprobes/my_wprobe/enable; then > > + echo "Failed to enable wprobe event" > > + exit_fail > > +fi > > [Severity: Medium] > Similarly, if enabling the wprobe fails at the sysfs boundary, would this > echo cause an early exit and bypass the diagnostic error message here as well? Also, this is a kind of double check. Even if writing 1 to enable succeeded, it is failed to set internal flag. This checks such problem. > > [ ... ] > > > +echo "-:my_wprobe" >> dynamic_events > > + > > +if grep -q my_wprobe dynamic_events; then > > + echo "Failed to remove wprobe event" > > + exit_fail > > +fi > > [Severity: Medium] > Could this echo also trigger an early exit if removing the wprobe fails, > bypassing the failure message? Ditto. Thanks, -- Masami Hiramatsu (Google) <[email protected]>
