On Tue, 2024-11-05 at 22:40 +0106, John Ogness wrote: > On 2024-11-05, Marcos Paulo de Souza <mpdesouza@xxxxxxxx> 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@xxxxxxxxxxxxx> Thanks John!