Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 3 Jan 2023 15:04:55 +0100
From:   Petr Mladek <pmladek@suse.com>
To:     John Ogness <john.ogness@linutronix.de>
Cc:     Sergey Senozhatsky <senozhatsky@chromium.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH printk v3 5/6] printk: introduce
 console_get_next_message() and console_message
Message-ID: <Y7Q2B+p1gxJ1WCdj@alley>
References: <20221221202704.857925-1-john.ogness@linutronix.de>
 <20221221202704.857925-6-john.ogness@linutronix.de>
 <87bknva1jg.fsf@jogness.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87bknva1jg.fsf@jogness.linutronix.de>
Precedence: bulk

On Thu 2022-12-22 16:47:39, John Ogness wrote:
> On 2022-12-21, John Ogness <john.ogness@linutronix.de> wrote:
> > +static bool console_emit_next_record(struct console *con, bool *handover, int cookie)
> > +{
> > +	bool is_extended = console_srcu_read_flags(con) & CON_EXTENDED;
> > +	static char dropped_text[DROPPED_TEXT_MAX];
> > +	static struct console_buffers cbufs;
> > +	static struct console_message cmsg = {
> > +		.cbufs = &cbufs,
> > +	};
> 
> @cmsg should not be static. The whole point of the console_message
> wrapper struct is so that it can sit on the stack.

Well, it might actually be better to keep it static for now.
It always points to static struct console_buffers cbufs anyway.

It would make sense to have it on stack only when it uses
different buffers.

Which brings the question. Does it makes sense to use
the same buffers by different struct console_message?
Will it be safe in any situation?

I did not want to complicate it yesterday. I think that
I have already proposed this. But this brings back
the question whether it makes sense to have two structures
at all.

I still think that it would be easier and even more safe
to put everything into struct console_message.

I mean to have:

struct console_message {
	char			buf[CONSOLE_EXT_LOG_MAX];
	char			scratch_buf[LOG_LINE_MAX];
	unsigned int		len;
	u64			seq;
	unsigned long		dropped;
};

Best Regards,
Petr