Re: [PATCH v2] printk: Remove console options before decoding the name

[Petr Mladek <pmladek@suse.com>]

On Thu 2026-09-17 09:05:51, David Engraf wrote:
> This fixes a regression when a console option includes ':'. Commit
> 7640f1a44eba ("printk: Add match_devname_and_update_preferred_console()")
> introduced console=DEVNAME:0.0 hardware style addressing by looking for a
> colon. If the colon is part of an option the name is handled as devname
> instead of ttyname.
> 
> Fix by handling the options first which will add a NULL terminator to the
> string.
> 
> Signed-off-by: David Engraf <david.engraf@sysgo.com>
> ---
>  kernel/printk/printk.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 6d3d18a50da74..f4803fe05a0aa 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2646,24 +2646,25 @@ static int __init console_setup(char *str)
>  	if (_braille_console_setup(&str, &brl_options))
>  		return 1;
>  
> +	/*
> +	 * Decode str into name, index and options. Start with options, since
> +	 * it might also contain a ':' used for DEVNAME.
> +	 */
> +	options = strchr(str, ',');
> +	if (options)
> +		*(options++) = 0;
> +
>  	/* For a DEVNAME:0.0 style console the character device is unknown early */
>  	if (strchr(str, ':'))
>  		devname = buf;
>  	else
>  		ttyname = buf;
>  
> -	/*
> -	 * Decode str into name, index, options.
> -	 */
>  	if (ttyname && isdigit(str[0]))
>  		scnprintf(buf, sizeof(buf), "ttyS%s", str);
>  	else
>  		strscpy(buf, str);

Sashiko AI has the following comment:

| Does moving the options parsing and null-termination earlier in this function
| leave the loop below with an unreachable condition?
| 
| Since str is now truncated at the first comma before being copied into buf,
| buf will never contain a comma. This means the comma check inside the loop
| over buf appears to be structurally impossible to satisfy:
| 
| 	for (s = buf; *s; s++)
| 		if ((ttyname && isdigit(*s)) || *s == ',')
| 			break;
| 
| Can the comma check be safely removed from the loop condition?

And it is right. The original code copied the original string into
"buf". The new does not copy the options any longer.

It would deserve some refactoring to make the code cleaner.
Something like:

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index f4803fe05a0a..966744fb4bcc 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2672,17 +2672,18 @@ static int __init console_setup(char *str)
 		strscpy(buf, "ttyS1");
 #endif
 
-	for (s = buf; *s; s++)
-		if ((ttyname && isdigit(*s)) || *s == ',')
-			break;
-
-	/* @idx will get defined when devname matches. */
-	if (devname)
-		idx = -1;
-	else
+	if (ttyname) {
+		/* Detect @idx in ttyname and remove it. */
+		for (s = buf; *s; s++) {
+			if (isdigit(*s))
+				break;
+		}
 		idx = simple_strtoul(s, NULL, 10);
-
-	*s = 0;
+		*s = 0;
+	} else {
+		/* @idx will get defined when devname matches. */
+		idx = -1;
+	}
 
 	__add_preferred_console(ttyname, idx, devname, options, brl_options, true);
 	return 1;


I see two possibilities. We could either merge this cleanup into the
original patch and send v3. Or we could add it on top of the original
patch.

I would slightly prefer v3 and have both changes in a single patch.

Best Regards,
Petr