Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Unconditional full read of the trace buffer risks memory exhaustion and test hangs. - [Low] Flawed shell redirection fails to silence stderr in `ifrmmod`. --
commit b7cfa1be78853b6d269ce0f8b3593fb8481c478f Author: Jim Cromie <[email protected]> selftests-dyndbg: add a dynamic_debug run_tests target This commit adds a selftest script for dynamic-debug and includes it in the selftests Makefile. The script currently has basic tests which modify pr_debug() flags in the builtin params module. > diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > new file mode 100755 > index 0000000000000..1239f1e105911 > --- /dev/null > +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh [ ... ] > +function error_log_ref { > + # to show what I got > + : echo "# error-log-ref: $1" > + : echo cat \$2 > +} > + > +function ifrmmod { > + lsmod | grep "$1" 2>&1>/dev/null && rmmod $1 > +} [Severity: Low] Are the shell redirections in the correct order in ifrmmod()? Bash evaluates redirections left-to-right, so `2>&1` first redirects stderr to the original stdout, and `>/dev/null` then redirects stdout to `/dev/null`. This causes any stderr produced by the command to leak to the terminal instead of being properly silenced. Should this use `>/dev/null 2>&1` instead? [ ... ] > +# $1 - trace instance name, 0 for global event trace > +# $2 - line number counting from the bottom > +# $3 - text to search for > +function search_trace_name() { > + if [ "$1" = "0" ]; then > + buf=$(cat /sys/kernel/tracing/trace) [Severity: Medium] Will this unconditional read of the entire trace buffer risk memory exhaustion? Ftrace buffers can grow to hundreds of megabytes or gigabytes, and reading them entirely into bash memory risks OOM kills or severe performance degradation. Additionally, if a specific line is requested for checking (when `$2 != 0`), the `buf` variable is populated but ignored during the search evaluation, making the expensive read entirely useless. Although the commit message notes these functions will be used shortly, could this logic flaw cause test hangs when the function is utilized? [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=22
