Re: [PATCH v13 30/39] dyndbg: split multi-query strings with @

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] dyndbg: ddebug_find_delimiter() tracks quotes across comments and newlines
--

commit 0c538c4aea6cd34ef1fce80f637f8815b5c63761
Author: Jim Cromie <jim.cromie@gmail.com>

dyndbg: split multi-query strings with @

This commit adds '@' as a multi-command delimiter alongside ';' and '\n' in
dynamic debug queries, using a quote-aware helper ddebug_find_delimiter() to
avoid splitting on delimiters inside quoted strings. It also updates the
dynamic debug documentation and selftests accordingly.

> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 5d99c3aef5a16..11f7350f35907 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -622,6 +622,30 @@ static int ddebug_exec_query(char *query_string, const char *modname)
>  	return nfound;
>  }
>  
> +/*
> + * Split multi-query string on delimiters (@, ;, \n), ignoring delimiters
> + * that appear inside single or double quoted strings.
> + */
> +static char *ddebug_find_delimiter(char *str, const char *delims)
> +{
> +	bool in_quote = false;
> +	char quote_char = 0;
> +
> +	for (; *str; str++) {
> +		if (*str == '"' || *str == '\'') {
> +			if (!in_quote) {
> +				in_quote = true;
> +				quote_char = *str;
> +			} else if (*str == quote_char) {
> +				in_quote = false;
> +			}
> +		} else if (!in_quote && strchr(delims, *str)) {
> +			return str;
> +		}
> +	}
> +	return NULL;
> +}
> +
>  /* handle multiple queries in query string, continue on error, return
>     last error or number of matching callsites.  Module name is either
>     in the modname arg (for boot args) or perhaps in query string.
> @@ -632,7 +656,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 = ddebug_find_delimiter(query, "@;\n");
>  		if (split)
>  			*split++ = '\0';

[Severity: Medium]
Can ddebug_find_delimiter() drop or merge subsequent lines in a multi-line
query when a '#' comment contains an apostrophe or when a line has an
unclosed quote?

Previously, ddebug_exec_queries() called strpbrk(query, ";\n"), which
unconditionally split the input buffer on '\n' before comment handling in
ddebug_exec_queries() and ddebug_tokenize():

lib/dynamic_debug.c:ddebug_exec_queries() {
	...
	query = skip_spaces_and_commas(query);

	if (!query || !*query || *query == '#')
		continue;
	...
}

lib/dynamic_debug.c:ddebug_tokenize() {
	...
	while (*buf) {
		...
		if (*buf == '#')
			break;	/* token starts comment, skip rest of line */
	...
}

With ddebug_find_delimiter(query, "@;\n"), quote state is tracked across '#'
comments and across '\n'. If a multi-line write to
/proc/dynamic_debug/control contains an apostrophe inside a comment, such as:

  # don't enable foo
  func bar +p

or an inline comment:

  func foo +p # module's debug
  func bar +p

ddebug_find_delimiter() sets in_quote = true on the '\'' and ignores the '\n'
at the end of the comment line. As a result, "func bar +p" is merged into the
commented line and silently discarded when ddebug_exec_queries() or
ddebug_tokenize() skips the comment.

Similarly, if a line has an unclosed quote, ddebug_find_delimiter() no longer
splits at '\n'. On a multi-line buffer, subsequent lines are concatenated
into the broken line instead of failing that line and continuing. On a
single-line write via echo, the trailing '\n' is left attached to the query
string and emitted in the v2pr_info() and pr_err("unclosed quote: %s\n", buf)
messages, which also appears to be why the golden hashes for the unclosed
quote tests (FT_grammar_errs.1, FT_grammar_errs.21, and FT_grammar_errs.41)
changed in GOLDEN_RECORDS in
tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh.

Should '\n' always terminate a query (and reset quote state, or be split
before scanning for ';' and '@'), and should '#' comments outside quotes skip
to the end of the line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027-dd-cmap-part2-clean-v13-0-9267923be612@gmail.com?part=30