Re: [PATCH v4] ALSA: control: Fix parsing of numeric fields

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ALSA: control_led: unconsumed closing quote in parse_string()
  stops parsing
--

commit ca72582941d6fb9701c15b50af83f1b948e9bd29
Author: Sreeraj S Kurup <sreekuttan2156239@gmail.com>

ALSA: control: Fix parsing of numeric fields

This commit updates find_eos(), parse_uint(), and parse_string() so they
return the end of the current substring rather than advancing past the
delimiter, and NUL-terminates the numeric substring before calling
kstrtouint().

> diff --git a/sound/core/control_led.c b/sound/core/control_led.c
> index 3d55da3af2a05..8cfd305fc1d2a 100644
> --- a/sound/core/control_led.c
> +++ b/sound/core/control_led.c
[ ... ]
> @@ -491,8 +492,6 @@ static char *parse_string(char *s, char *val, size_t val_size)
>  		}
>  	}
>  	*val = '\0';
> -	if (*s)
> -		s++;
>  	return s;
>  }

[Severity: Medium]
Does removing "if (*s) s++;" at the end of parse_string() leave s pointing
at the closing quote when parsing a quoted string?

In the quoted-string branch, the loop stops when *s matches the opening
quote character:

sound/core/control_led.c:parse_string() {
	if (*s == '"' || *s == '\'') {
		char c = *s;
		s++;
		while (*s && *s != c) {
			...
			s++;
		}
	} else {
...

Previously, the trailing "if (*s) s++;" stepped past the closing quote so
that s pointed at the following comma (or NUL), and set_led_id() then
stepped past that comma.  With it removed, a quoted name followed by
another field (such as "name='Master Playback Switch',index=1") returns a
pointer to the closing quote, causing set_led_id() to miss the comma and
break out of its loop without parsing any remaining fields.  Advancing
past the closing quote inside the quoted-string branch would preserve the
expected return position.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109155941.5850-1-sreekuttan2156239@gmail.com?part=1