[PATCH v2 24/59] dyndbg: treat comma as a token separator

[Louis Chauvet]

Le 20/03/2025 à 19:52, Jim Cromie a écrit :
> Treat comma as a token terminator, just like a space.  This allows a
> user to avoid quoting hassles when spaces are otherwise needed:
> 
>   :#> modprobe drm dyndbg=class,DRM_UT_CORE,+p\;class,DRM_UT_KMS,+p
> 
> or as a boot arg:
> 
>   drm.dyndbg=class,DRM_UT_CORE,+p  # todo: support multi-query here
> 
> Given the many ways a boot-line +args can be assembled and then passed
> in/down/around shell based tools, this may allow side-stepping all
> sorts of quoting hassles thru those layers.
> 
> existing query format:
> 
>   modprobe test_dynamic_debug dyndbg="class D2_CORE +p"
> 
> new format:
> 
>   modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
> 
> Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
> Co-developed-by: Łukasz Bartosik <ukaszb at chromium.org>
> Signed-off-by: Łukasz Bartosik <ukaszb at chromium.org>

This patch seems to be unrelated to the whole series, and can be applied 
on the current rc. What do you think about extracting the patches 24, 
25, 26, 27? I understand 28 depends on it, but I believe 24..27 can be 
merged before as they are simple.

> ---
>   lib/dynamic_debug.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index cd3eec5bb81c..168663629ef2 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -293,6 +293,14 @@ static int ddebug_change(const struct ddebug_query *query, struct flag_settings
>   	return nfound;
>   }
>   
> +static char *skip_spaces_and_commas(const char *str)
> +{
> +	str = skip_spaces(str);
> +	while (*str == ',')
> +		str = skip_spaces(++str);
> +	return (char *)str;
> +}
> +

This is a bit complex to read, maybe you can simply re-write skip_space 
completely to avoid the explicit check against is_skip below

bool is_skip(char c) {
	return isspace(c) || c == ',';
}
char *skip(char *s) {
	while (is_skip(*s))
		s++;
	return s;
}

With or without this:

Reviewed-by: Louis Chauvet <louis.chauvet at booltin.com>

>   /*
>    * Split the buffer `buf' into space-separated words.
>    * Handles simple " and ' quoting, i.e. without nested,
> @@ -306,8 +314,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);
>   		if (!*buf)
>   			break;	/* oh, it was trailing whitespace */
>   		if (*buf == '#')
> @@ -323,7 +331,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++)

                                                    ^ here use is_skip

>   				;
>   			if (end == buf) {
>   				pr_err("parse err after word:%d=%s\n", nwords,
> @@ -595,7 +603,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
>   		if (split)
>   			*split++ = '\0';
>   
> -		query = skip_spaces(query);
> +		query = skip_spaces_and_commas(query);
> +
>   		if (!query || !*query || *query == '#')
>   			continue;
>   

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