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

[Takashi Iwai <tiwai@suse.de>]

On Sun, 08 Nov 2026 13:48:32 +0100,
Sreeraj S Kurup wrote:
> 
> parse_uint() passes the remaining input string to kstrtoull().
> 
> When the input contains comma-separated fields, the string
> contains both the numeric value and the following fields.
> 
> For example, with:
> 
> device=1,subdevice=0,name=Master
> 
> kstrtoull() receives "1,subdevice=0,name=Master" and fails
> to parse the numeric value.
> 
> Stop parsing at the next comma and pass only the numeric
> field to kstrtouint().
> 
> This allows numeric fields to be parsed correctly in
> comma-separated input.
> 
> Fixes: a135dfb5de15 ("ALSA: led control - add sysfs kcontrol LED marking layer")
> Signed-off-by: Sreeraj S Kurup <sreekuttan2156239@gmail.com>
> ---
> v3:
>  - No code changes. Retain the v2 implementation after
>    comparing the alternative approaches.
> v2:
>  - Add the Fixes trailer.

I think a bit more easier to understand the code flow would to make
each parse_*() function returning the substring end position, instead
of the next substring.  (BTW, this fits more with the function name
find_eos(). too).

The main loop in set_led_id() already treats it, 

	while (*s) {
		os = s;	
		....
		if (*s == ',')
			s++;
		if (s == os)
			break;
	}

Both parse_uint() and parse_iface() return find_eos() value as is.
parse_string() has an extra increment for the next substring, so we
just need to drop it.

That is, what we need to change is something like:

--- a/sound/core/control_led.c
+++ b/sound/core/control_led.c
@@ -451,22 +451,24 @@ static const struct attribute_group *snd_ctl_led_dev_attr_groups[] = {
 	NULL,
 };
 
+/* return the end of substring pointing to either comma or NUL */
 static char *find_eos(char *s)
 {
 	while (*s && *s != ',')
 		s++;
-	if (*s)
-		s++;
 	return s;
 }
 
 static char *parse_uint(char *s, unsigned int *val)
 {
-	unsigned long long res;
-	if (kstrtoull(s, 10, &res))
-		res = 0;
-	*val = res;
-	return find_eos(s);
+	char *end = find_eos(s);
+	char delimiter = *end;
+
+	*end = '\0';
+	if (kstrtouint(s, 10, val))
+		*val = 0;
+	*end = delimiter;
+	return end;
 }
 
 static char *parse_string(char *s, char *val, size_t val_size)
@@ -491,8 +493,6 @@ static char *parse_string(char *s, char *val, size_t val_size)
 		}
 	}
 	*val = '\0';
-	if (*s)
-		s++;
 	return s;
 }
 

thanks,

Takashi