linux-kernel - Re: [PATCH v2 1/2] printk: Introduce FORCE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <32c882534566f0a9ef578cc0fef20e5aab77eee6.camel@suse.com>
Date: Tue, 05 Nov 2024 22:04:24 -0300
From: Marcos Paulo de Souza <mpdesouza@...e.com>
To: John Ogness <john.ogness@...utronix.de>, Petr Mladek <pmladek@...e.com>,
  Steven Rostedt <rostedt@...dmis.org>, Sergey Senozhatsky
 <senozhatsky@...omium.org>, Greg Kroah-Hartman	
 <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-serial@...r.kernel.org
Subject: Re: [PATCH v2 1/2] printk: Introduce FORCE_CON flag

On Tue, 2024-11-05 at 22:40 +0106, John Ogness wrote:
> On 2024-11-05, Marcos Paulo de Souza <mpdesouza@...e.com> wrote:
> > @@ -2947,6 +2953,7 @@ bool printk_get_next_message(struct
> > printk_message *pmsg, u64 seq,
> >  	struct printk_info info;
> >  	struct printk_record r;
> >  	size_t len = 0;
> > +	bool force_con;
> >  
> >  	/*
> >  	 * Formatting extended messages requires a separate
> > buffer, so use the
> > @@ -2965,9 +2972,13 @@ bool printk_get_next_message(struct
> > printk_message *pmsg, u64 seq,
> >  
> >  	pmsg->seq = r.info->seq;
> >  	pmsg->dropped = r.info->seq - seq;
> > +	force_con = r.info->flags & LOG_FORCE_CON;
> >  
> > -	/* Skip record that has level above the console loglevel.
> > */
> > -	if (may_suppress && suppress_message_printing(r.info-
> > >level))
> > +	/*
> > +	 * Skip records that are not forced to be printed on
> > consoles and that
> > +	 * has level above the console loglevel.
> > +	 */
> > +	if (!force_con && may_suppress &&
> > suppress_message_printing(r.info->level))
> >  		goto out;
> 
> Rather than adding a new local variable, setting it, and expanding
> the
> condition, it might be cleaner to just update @may_suppress before
> the
> condition check?
> 
> 	/* Records forced to be printed on consoles must not be
> skipped. */
> 	may_suppress &= !(r.info->flags & LOG_FORCE_CON);

Well, your suggestion seems clever than what I did :)

IHMO, I would prefer the new variable as it's easier to read (for me at
least), but I can change if you think it's better..

> 
> Feel free to ignore this suggestion if you think having an extra
> variable is easier to follow.
> 
> With or without suggested change:
> 
> Reviewed-by: John Ogness <john.ogness@...utronix.de>

Thanks John!