Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] selftests: classmap inheritance hash check excludes submod - [Low] selftests: undocumented uncommenting of grammar testcase --
commit 7d522a72bd526c1e41ef1a1e604e466e1e729843 Author: Jim Cromie <[email protected]> selftests/dyndbg: Enable FT_classmap_inheritance Exercise classmap state inheritance between supermod and submod, verifying that class settings and controlling parameter states propagate from parent classmap definitions to dependent class users upon module load and during runtime reconfiguration. > diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > index c279c795867b6..5a805cb30e6cd 100755 > --- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh > +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh [ ... ] > @@ -189,7 +190,7 @@ function FT_grammar_ok { > > # use 4 keywords (max 9 words inc flags) > ddcmd "module foo file bar.c func buz class D2_CORE +_" # 4 keywords > - #ddcmd "module foo file bar.c func buz class D2 line 100 +_" # 5 keywords > + ddcmd "module foo file bar.c func buz class D2 line 100 +_" # 5 keywords [Severity: Low] Was this undocumented uncommenting of a grammar testcase intentional? It modifies the behavior of an existing, separate test in FT_grammar_ok without documenting it in the commit message. [ ... ] > @@ -291,6 +292,78 @@ function FT_basic_queries { > ddcmd "file $f =_" "$f" > } > > +function FT_classmap_inheritance { [ ... ] > + # Real-time mathematical proof that load-time (modprobe) parameter > parsing > + # and runtime (sysfs write) parameter configurations are perfectly > equivalent! > + local hash_modprobe=$(slice_and_hash_ddctrl '\[test_dynamic_debug\]') > + > + # Fresh load with default parameters, then configure them dynamically at > runtime > + ifrmmod test_dynamic_debug_submod > + ifrmmod test_dynamic_debug > + my_modprobe test_dynamic_debug > + my_modprobe test_dynamic_debug_submod > + echo 0x16 > /sys/module/test_dynamic_debug/parameters/p_disjoint_bits > + echo 5 > /sys/module/test_dynamic_debug/parameters/p_level_num > + > + local hash_sysfs=$(slice_and_hash_ddctrl '\[test_dynamic_debug\]') [Severity: Medium] Does this classmap inheritance hash check exclude the submodule? Because of the strict regex using escaped brackets, the grep strictly matches the parent module's section and filters out all [test_dynamic_debug_submod] lines. Consequently, the hash equivalence check only proves the parent module responds correctly, ignoring the dependent submodule whose inheritance is supposedly being tested in FT_classmap_inheritance. > + if [ "$hash_modprobe" != "$hash_sysfs" ]; then > + echo -e "${RED}: Load-time vs runtime parameter equivalence check > failed!${NC}" > + exit $ksft_fail > + else > + v_echo "${GREEN}: Proven: parameter load-time (modprobe) " \ > + "and runtime (sysfs write) are equivalent!${NC}" > + fi -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=24
