On Wed, Sep 23, 2026 at 4:46 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [High] dyndbg: UAPI break for unquoted commas in queries
> --
>
> commit 3647cd55c26b2f7588078b414e3120a9276258a8
> Author: Jim Cromie <[email protected]>
>
> dyndbg: control-parser: treat comma as a token separator
>
> This commit updates the dynamic debug control parser to treat commas as
> token separators alongside spaces and tabs. This is intended to allow users
> to avoid shell quoting issues by substituting spaces with commas.
>
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > index 8eef2e7387a98..6482fd28def46 100644
> > --- a/lib/dynamic_debug.c
> > +++ b/lib/dynamic_debug.c
> [ ... ]
> > @@ -336,8 +344,8 @@ static int ddebug_tokenize(char *buf, char *words[],
> > int maxwords)
> > while (*buf) {
> > char *end;
> >
> > - /* Skip leading whitespace */
> > - buf = skip_spaces(buf);
> > + /* Skip leading whitespace and comma */
> > + buf = skip_spaces_and_commas(buf);
>
> [Severity: High]
> Does this introduce a UAPI regression for existing unquoted queries that
> contain commas?
>
> For example, if a user currently submits a query like "format a,b +p" via
> the debugfs interface, the parser would previously see three tokens: "format",
> "a,b", and "+p".
>
> With commas treated as separators here and in the loop below, wouldn't this
> result in four disconnected tokens: "format", "a", "b", and "+p"?
>
Treating comma as a lexical delimiter alongside space and tab is an
intentional grammar extension, explicitly documented in
Documentation/admin-guide/dynamic-debug-howto.rst:
0. Comma separation exists specifically to solve shell and command-line
quoting friction in space-hostile contexts (boot command lines and
modprobe args like "modprobe drm dyndbg=class,DRM_UT_CORE,+p").
1. The dynamic debug grammar rule is uniform: any argument containing a
lexical delimiter must be quoted. Spaces inside format strings have
always required quotes (e.g. format "foo bar" +p). Because commas are
now full token terminators, commas inside format arguments must also
be quoted (e.g. format "a,b" +p).
and the user is free to add the quotes if literal "a,b" is what they wanted.
#> echo format "a,b" +p >control
> > if (!*buf)
> > break; /* oh, it was trailing whitespace */
> > if (*buf == '#')
> > @@ -353,7 +361,7 @@ static int ddebug_tokenize(char *buf, char *words[],
> > int maxwords)
> > return -EINVAL; /* unclosed quote */
> > }
> > } else {
> > - for (end = buf; *end && !isspace(*end); end++)
> > + for (end = buf; *end && !isspace(*end) && *end !=
> > ','; end++)
> > ;
>
> [Severity: High]
> Because the parser now explicitly stops at commas within an unquoted string,
> existing queries containing unquoted commas will be split.
>
> Since ddebug_parse_query() expects an even number of match-spec tokens
> (checking nwords % 2 != 0), won't it fail and print "expecting pairs of
> match-spec <value>"?
>
> This would break automated scripts or user commands that pass unquoted
> commas in their match arguments. Is there a way to support commas as
> separators without breaking existing unquoted comma usage?
again, if theyre looking for "a,b", then they should quote it.
just like if they were looking for "a b"
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=29
