kernel/printk/printk.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-)
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 | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 6d3d18a50da74..c297a0ae04f7b 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2646,24 +2646,22 @@ static int __init console_setup(char *str)
if (_braille_console_setup(&str, &brl_options))
return 1;
+ /* Decode str into name, index, options */
+ 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);
- options = strchr(str, ',');
- if (options)
- *(options++) = 0;
-
#ifdef __sparc__
if (!strcmp(str, "ttya"))
strscpy(buf, "ttyS0");
--
2.53.0
On Wed, Sep 16, 2026 at 08:32:18AM +0300, David Engraf wrote: > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -2646,24 +2646,22 @@ static int __init console_setup(char *str) > if (_braille_console_setup(&str, &brl_options)) > return 1; > > + /* Decode str into name, index, options */ > + options = strchr(str, ','); > + if (options) > + *(options++) = 0; > + How about update the comment for why it needs to be first? Maybe something like: Decode str into options first. The options may contain a ':' used also for DEVNAME.
On 16.09.26 08:48 wrote Tony Lindgren: > On Wed, Sep 16, 2026 at 08:32:18AM +0300, David Engraf wrote: >> --- a/kernel/printk/printk.c >> +++ b/kernel/printk/printk.c >> @@ -2646,24 +2646,22 @@ static int __init console_setup(char *str) >> if (_braille_console_setup(&str, &brl_options)) >> return 1; >> >> + /* Decode str into name, index, options */ >> + options = strchr(str, ','); >> + if (options) >> + *(options++) = 0; >> + > > How about update the comment for why it needs to be first? > > Maybe something like: > > Decode str into options first. The options may contain a ':' used also > for DEVNAME. Okay I can update the comment if there are no other objections. Best regards - David
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);
- options = strchr(str, ',');
- if (options)
- *(options++) = 0;
-
#ifdef __sparc__
if (!strcmp(str, "ttya"))
strscpy(buf, "ttyS0");
--
2.53.0
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
On Wed, Sep 23, 2026 at 11:37:03AM +0200, Petr Mladek wrote:
> 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.
OK
> 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;
Nice, you could now initialize idx = -1 to start with to leave out the
else for setting devname idx?
> 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.
Having a v3 sounds good to me.
On Wed 2026-09-23 13:19:15, Tony Lindgren wrote:
> On Wed, Sep 23, 2026 at 11:37:03AM +0200, Petr Mladek wrote:
> > 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.
> > >
> > > --- 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.
>
> OK
>
> > 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;
>
> Nice, you could now initialize idx = -1 to start with to leave out the
> else for setting devname idx?
I would personally prefer to keep the else part because it makes it
clear how the "devname" variant is handled. But I could live without
it as well.
> > 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.
>
> Having a v3 sounds good to me.
Great.
Best Regards,
Petr
On Wed, Sep 23, 2026 at 02:06:26PM +0200, Petr Mladek wrote: > On Wed 2026-09-23 13:19:15, Tony Lindgren wrote: > > Nice, you could now initialize idx = -1 to start with to leave out the > > else for setting devname idx? > > I would personally prefer to keep the else part because it makes it > clear how the "devname" variant is handled. But I could live without > it as well. OK thanks, that works just fine for me.
On 23.09.26 15:14 wrote Tony Lindgren: > On Wed, Sep 23, 2026 at 02:06:26PM +0200, Petr Mladek wrote: >> On Wed 2026-09-23 13:19:15, Tony Lindgren wrote: >>> Nice, you could now initialize idx = -1 to start with to leave out the >>> else for setting devname idx? >> >> I would personally prefer to keep the else part because it makes it >> clear how the "devname" variant is handled. But I could live without >> it as well. > > OK thanks, that works just fine for me. Thanks for the review. I'm going to prepare v3 including Petr's changes. Best regards - David
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 and refactor idx handling to clean up the code (thanks to Petr
Mladek).
Signed-off-by: David Engraf <david.engraf@sysgo.com>
---
kernel/printk/printk.c | 36 +++++++++++++++++++-----------------
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 6d3d18a50da74..2cbb84effb619 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);
- options = strchr(str, ',');
- if (options)
- *(options++) = 0;
-
#ifdef __sparc__
if (!strcmp(str, "ttya"))
strscpy(buf, "ttyS0");
@@ -2671,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 = ttyname; *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;
--
2.53.0
On Thu, Sep 24, 2026 at 10:00:57AM +0300, 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 and refactor idx handling to clean up the code (thanks to Petr
> Mladek).
Maybe clarify the above a bit to make it clear that the refactoring is
needed for the fix. Something like:
Fix by handling the options first which will add a NULL terminator to the
string. Note that parsing the options first means that also idx parsing
needs changing. Handle the idx parsing by refactoring the code (thanks to
Petr Mladek).
Other than that:
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
On Thu, Sep 17, 2026 at 09:05:51AM +0300, 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.
Just curious, which console did you hit this issue with?
In any case, thanks for updating the comments:
Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
On 17.09.26 17:03 wrote Tony Lindgren:
> On Thu, Sep 17, 2026 at 09:05:51AM +0300, 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.
>
> Just curious, which console did you hit this issue with?
It's a self-developed console driver to access our hypervisor.
> In any case, thanks for updating the comments:
>
> Reviewed-by: Tony Lindgren <tony.lindgren@linux.intel.com>
Thanks
- David
On Fri, Sep 18, 2026 at 09:27:03AM +0300, David Engraf wrote:
> On 17.09.26 17:03 wrote Tony Lindgren:
> > On Thu, Sep 17, 2026 at 09:05:51AM +0300, 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.
> >
> > Just curious, which console did you hit this issue with?
>
> It's a self-developed console driver to access our hypervisor.
OK thanks. From a "fix or feature" point of view, I wonder if this issue
can happen with some of the current Linux console drivers too?
On 21.09.26 08:17 wrote Tony Lindgren:
> On Fri, Sep 18, 2026 at 09:27:03AM +0300, David Engraf wrote:
>> On 17.09.26 17:03 wrote Tony Lindgren:
>>> On Thu, Sep 17, 2026 at 09:05:51AM +0300, 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.
>>>
>>> Just curious, which console did you hit this issue with?
>>
>> It's a self-developed console driver to access our hypervisor.
>
> OK thanks. From a "fix or feature" point of view, I wonder if this issue
> can happen with some of the current Linux console drivers too?
AFAIK there is no restriction using ':' in the console options even if I
don't know any in-kernel driver using it. That's why I would say it's a
bug fix.
Best regards
- David
© 2016 - 2026 Red Hat, Inc.