Re: [PATCH v2 26/59] dyndbg: split multi-query strings with %

[Louis Chauvet <louis.chauvet@bootlin.com>]

Le 20/03/2025 à 19:52, Jim Cromie a écrit :
> Since commit
> 85f7f6c0edb8 ("dynamic_debug: process multiple debug-queries on a line")
> 
> Multi-query commands have been allowed:
> 
>    modprobe drm dyndbg="class DRM_UT_CORE +p; class DRM_UT_KMS +p"
>    modprobe drm dyndbg=<<EOX
>       class DRM_UT_CORE +p
>       class DRM_UT_KMS +p
>    EOX
> 
> More recently, the need for quoting was avoided by treating a comma
> like a space/token-terminator:
> 
>    modprobe drm dyndbg=class,DRM_UT_CORE,+p\;class,DRM_UT_KMS,+p
> 
> That works, but it needs the escaped semicolon, which is a shell
> special-char (one of the bash control operators), so it is brittle
> when passed in/down/around scripts.  In particular, it fails when
> passed to vng (virtme-ng).
> 
> So 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.
> 
> NOTE: it does break format matching on '%' patterns:
> 
> bash-5.2# ddcmd 'format "find-me: %foo" +p'
> [  203.900581] dyndbg: read 26 bytes from userspace
> [  203.900883] dyndbg: query 0: "format "find-me: " mod:*
> [  203.901118] dyndbg: unclosed quote: find-me:
> [  203.901355] dyndbg: tokenize failed
> [  203.901529] dyndbg: query 1: "foo" +p" mod:*
> [  203.901957] dyndbg: split into words: "foo"" "+p"
> [  203.902243] dyndbg: op='+' flags=0x1 maskp=0xffffffff
> [  203.902458] dyndbg: expecting pairs of match-spec <value>
> [  203.902703] dyndbg: query parse failed
> [  203.902871] dyndbg: processed 2 queries, with 0 matches, 2 errs
> bash: echo: write error: Invalid argument
> 
> The '%' splits the input into 2 queries, and both fail.  Given the
> limited utility of matching against the working parts of a format
> string "foo: %d bar %s", nothing is actually lost here.
> 
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
>   lib/dynamic_debug.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 168663629ef2..c44502787c2b 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -599,7 +599,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");
>   		if (split)
>   			*split++ = '\0';
>   

-- 
Louis Chauvet, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com