Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Arvind Sankar <nivedita@alum.mit.edu>
Date:   Thu, 21 May 2020 11:59:31 -0400
To:     Joe Perches <joe@perches.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        Chenggang Wang <wangchenggang@vivo.com>,
        linux-kernel@vger.kernel.org, Petr Mladek <pmladek@suse.com>,
        Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
        Steven Rostedt <rostedt@goodmis.org>
Subject: Re: [RFC PATCH 2/2] init: Allow multi-line output of kernel command
 line
Message-ID: <20200521155931.GA568639@rani.riverdale.lan>
References: <cover.1589916689.git.joe@perches.com>
 <2b3832fed9370f0f8dfd1ea33dddb1d05a36e265.1589916689.git.joe@perches.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <2b3832fed9370f0f8dfd1ea33dddb1d05a36e265.1589916689.git.joe@perches.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, May 19, 2020 at 12:42:35PM -0700, Joe Perches wrote:
> ARM may have its longest possible command line larger than the longest
> possible printk.
> 
> If necessary, emit the commend line on multiple lines.
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> 
> compiled, untested
> 
>  init/main.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/init/main.c b/init/main.c
> index b63a3c001ac4..b3ebbbc129ae 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -826,6 +826,34 @@ void __init __weak arch_call_rest_init(void)
>  	rest_init();
>  }
>  
> +static void __init print_cmdline(char *line)
> +{
> +#ifdef CONFIG_PRINTK
> +	const char *prefix = "Kernel command line";
> +	size_t len = strlen(line);
> +
> +	while (len > PRINTK_LOG_LINE_MAX) {
> +		char *pos = line;
> +		char *last_pos = pos + PRINTK_LOG_LINE_MAX - 1;
> +		char saved_char;
> +		/* Find last space char within the maximum line length */
> +		while ((pos = memchr(pos, ' ', len - (pos - line))) &&
> +		       (pos - line) < PRINTK_LOG_LINE_MAX - 1) {
> +			last_pos = pos;
> +		}
> +		saved_char = line[last_pos - line];
> +		line[last_pos - line] = 0;
> +		pr_notice("%s: %s\n", prefix, line);
> +		prefix = "Kernel command line (continued)";
> +		line[last_pos - line] = saved_char;
> +		len -= pos - line;
> +		line += pos - line;
> +	}
> +
> +	pr_notice("%s: %s\n", prefix, line);
> +#endif

I might be missing something, but this seems broken:
(1) If there is a ' ', the memchr will set pos to the ' ' the first time
through the inner loop, and then go into an infinite loop? You want
memrchr here but the kernel doesn't seem to have one.
(2) If there are no remaining ' 's pos will be NULL and the last two
lines in the outer loop use it -- those should be last_pos instead?
(3) Once those are fixed, you need to ensure you make progress: if
there's exactly one ' ' remaining, at the very beginning of the
remaining line, you need to print upto the PRINTK_LOG_LINE_MAX, not 0
bytes.