Re: [PATCH v5 04/16] printk: Separate code for adding/updating preferred console metadata

[John Ogness <john.ogness@linutronix.de>]

On 2026-09-29, Petr Mladek <pmladek@suse.com> wrote:
> The logic for adding or updating a preferred console is currently
> duplicated within __add_preferred_console(), making the code difficult
> to follow and prone to consistency issues.
>
> Split the code for adding or updating entries to avoid code duplication.
> Make it clear what operations are done only for new entries.
>
> Use this opportunity and replace an int with an unsigned int for
> iterating over the preferred_consoles array.
>
> It is a preparation step before adding more consistency checks.
> No intended functional changes.
>
> Signed-off-by: Petr Mladek <pmladek@suse.com>

Reviewed-by: John Ogness <john.ogness@linutronix.de>

Some minor comments below...

> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index c3e50f19a32b..174aee533a53 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2577,35 +2584,56 @@ static int __add_preferred_console(const char *name, const short idx,
>  	if (name && idx < 0)
>  		return -EINVAL;
>  
> +	if (!pc->name[0] && !pc->devname[0])
> +		is_new_entry = true;
> +
> +	if (is_new_entry) {
> +		if (devname)
> +			strscpy(pc->devname, devname);
> +		if (name)
> +			strscpy(pc->name, name);
> +		pc->index = idx;
> +	}
> +
>  	/*
> -	 *	See if this tty is not yet registered, and
> -	 *	if we have a slot free.
> +	 * Historically, the preferred options are stored only by the first
> +	 * caller who prefers the particular console.
>  	 */
> +	if (is_new_entry) {
> +		pc->options = options;
> +		braille_set_options(pc, brl_options);
> +	}
> +
> +	if (!brl_options)
> +		preferred_dev_console = i;
> +
> +	if (user_specified) {

Here you drop the comments:

	/*
	 * @pc console was defined by the user on the command line.
	 * Do not clear when added twice also by SPCR or the device tree.
	 */

> +		pc->user_specified = true;

And here you drop the comments:

	/* At least one console defined by the user on the command line. */

> +		console_set_on_cmdline = 1;
> +	}

The comments seem useful to help track all this. I do not see this
information preserved anywhere by the end of your series. But you have a
better understanding of the big picture. If you think the comments are
insignificant and/or misleading, I am fine with you dropping them.

John