Re: [PATCH 3/8] printk: Separate code for adding/updating preferred console metadata

[Chris Down]

Petr Mladek writes:
> @@ -2531,28 +2587,14 @@ static int __add_preferred_console(const char *name, const short idx,
> 	for (i = 0, pc = preferred_consoles;
> 	     i < MAX_PREFERRED_CONSOLES && (pc->name[0] || pc->devname[0]);
> 	     i++, pc++) {
> -		if ((name && strcmp(pc->name, name) == 0 && pc->index == idx) ||
> -		    (devname && strcmp(pc->devname, devname) == 0)) {
> -			if (!brl_options)
> -				preferred_dev_console = i;
> -			set_user_specified(pc, user_specified);
> -			return 0;
> -		}

So, in this old code when __add_preferred_console() finds an existing matching 
entry, it returns immediately without touching the options at all.

But in the new patch the loop breaks out and falls through to 
update_preferred_console(), which only conditionally updates options.

This has different behaviour. For example if you imagine SPCR/DT setting the 
options, before they win out against the command line, but now they do not.

This may well be intended, since the user cmdline probably should win over the 
platform defaults anyway. I'd argue that sounds like the right behaviour 
anyway. But I don't see this change being mentioned as intentional in the 
changelog. Is it?  If it is, let's mention it there.

With this clarified:

Acked-by: Chris Down <chris@xxxxxxxxxxxxxx>