Re: [PATCH v8 35/43] dyndbg: split multi-query strings with @
From: sashiko-bot@kernel.org
Date: 2026-09-05 18:24:32
Also in:
dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The selftest intended to verify the new '@' query separator mistakenly uses ';' instead, leaving the new feature untested. - [Medium] Unconditional string splitting on '@' breaks valid queries containing literal '@' characters (e.g., in format strings for device tree addresses or emails). -- commit 02bd61a3e52ea827cb76db337dc5c8fc31993564 Author: Jim Cromie [off-list ref] dyndbg: split multi-query strings with @ This patch adds '@' to the existing ';' and '\n' multi-command separators, which is more shell-friendly, so you can more fully avoid quoting and escaping hassles. It also updates the selftests script.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 6f700de9738c4..93a5a481c8b8c 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c@@ -631,7 +631,7 @@ static int ddebug_exec_queries(char *query, const char *modname) int i, errs = 0, exitcode = 0, rc, nfound = 0; for (i = 0; query; query = split) { - split = strpbrk(query, ";\n"); + split = strpbrk(query, "@;\n");
[Severity: Medium] Will this unconditional string splitting on '@' break valid queries containing literal '@' characters? For example, format strings containing device tree addresses or emails like dyndbg="format \"node@1000\" +p". It seems they will be incorrectly split at the '@' character before quote-aware tokenization occurs, causing syntax errors and failing to enable the intended debug statements.
if (split) *split++ = '\0';
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh index f0b18afa73722..0bb3c3e11df75 100755 --- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh +++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
[ ... ]
quoted hunk ↗ jump to hunk
@@ -325,9 +325,19 @@ function FT_basic_queries { ddcmd "module params +l" 'kernel/params.c' ddcmd "module params -m" 'kernel/params.c' ddcmd "module params =_" 'kernel/params.c' +} + +function FT_multi_query { + v_echo "${GREEN}# MULTI_QUERY_TESTS ${NC}" + if [ $LACK_DD_BUILTIN -eq 1 ]; then + echo "SKIP - test requires params, which is a builtin module" + return + fi + ddcmd =_ # zero everything - # multi-query commands split on ; on a single line + # multi-query commands on a single line, split on ;/@ respectively ddcmd "module params +mf ; module params func parse_args +sl" 'kernel/params.c' + ddcmd "module params -f ; module params func parse_args -l" 'kernel/params.c'
[Severity: Medium] Does this selftest command mistakenly use ';' instead of the intended '@' separator? The comment right above in FT_multi_query() indicates it tests splitting on ';/@ respectively', but both commands use ';', which means the new '@' separator feature remains completely untested here. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260905-dd-cmap-part2-clean-v8-0-a4cc0674f6fd@gmail.com?part=35