linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chris Down <chris@chrisdown.name>
To: Petr Mladek <pmladek@suse.com>
Cc: linux-kernel@vger.kernel.org,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	John Ogness <john.ogness@linutronix.de>,
	Johannes Weiner <hannes@cmpxchg.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Kees Cook <keescook@chromium.org>,
	kernel-team@fb.com
Subject: Re: output: was: Re: [PATCH v4] printk: Userspace format enumeration support
Date: Tue, 16 Feb 2021 16:52:39 +0000	[thread overview]
Message-ID: <YCv4V5EFeuEmyxSz@chrisdown.name> (raw)
In-Reply-To: <YCvqdrBc3wLDClhv@alley>

Hey Petr,

Petr Mladek writes:
>This produces something like:
>
>3Warning: unable to open an initial console.
>3Failed to execute %s (error %d)
>6Kernel memory protection disabled.
>3Starting init: %s exists but couldn't execute it (error %d)
>6Run %s as init process
>7initcall %pS returned %d after %lld usecs
>7calling  %pS @ %i
>2initrd overwritten (0x%08lx < 0x%08lx) - disabling it.
>
>The loglevel is not well separated. It is neither human readable
>nor safe for a machine processing . It works only for single digit.
>[...]
>It looks in less like: [...]

Hmm, why is it important that debugfs output is human readable? My impression 
was that it's fine to have machine-readable stuff there.

Re: not being not safe for machine processing because it only works for a 
single digit, I'm a little confused. KERN_* levels are, as far as I know, only 
a single byte wide, and we rely on that already (eg. in printk_skip_header()). 
We also already have precedent for null-separation/control characters in (for 
example) /proc/pid/cmdline.

What am I missing? :-)

>Well, it still might be non-trivial to find the string in the code
>and see what exactly has changed. It might be pretty useful
>to mention even the source_file:line, for example:
>
><3> init/main.c:1489: Warning: unable to open an initial console.\n
><3> init/main.c:1446: Failed to execute %s (error %d)\n
><6> init/main.c:1398: Kernel memory protection disabled.\n
><3> init/main.c:1366: Starting init: %s exists but couldn't execute it (error %d)\n

Almost certainly a theoretical concern, but I am not a big fan of this format, 
because it relies on a character (":") which is legal in paths (although as 
you'd expect, we don't have any cases in the kernel right now). That's one of 
the reasons why I preferred to use nulls, which can't be in a filename.

  reply	other threads:[~2021-02-16 16:53 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-12 15:30 [PATCH v4] printk: Userspace format enumeration support Chris Down
2021-02-12 18:01 ` kernel test robot
2021-02-13 14:29   ` Chris Down
2021-02-13 15:15     ` Chris Down
2021-02-16 15:53 ` output: was: " Petr Mladek
2021-02-16 16:52   ` Chris Down [this message]
2021-02-17 14:27     ` Petr Mladek
2021-02-17 15:28       ` Chris Down
2021-02-17 19:17         ` Steven Rostedt
2021-02-17 21:23           ` Chris Down
2021-02-18 11:34         ` Petr Mladek
2021-02-16 16:00 ` debugfs: " Petr Mladek
2021-02-16 17:18   ` Chris Down
2021-02-17 15:35     ` Petr Mladek
2021-02-17 15:49       ` Chris Down
2021-02-16 17:14 ` code style: " Petr Mladek
2021-02-16 17:27   ` Chris Down
2021-02-16 21:00     ` Johannes Weiner
2021-02-16 21:05       ` Chris Down
2021-02-17 15:45         ` Petr Mladek
2021-02-17 15:56           ` Chris Down
2021-02-18 10:58             ` Petr Mladek
2021-02-17 16:00           ` Johannes Weiner
2021-02-17 16:09     ` Petr Mladek
2021-02-17 16:25       ` Chris Down
2021-02-17 16:32         ` Chris Down
2021-02-18 10:45           ` Petr Mladek
2021-02-18 12:21 ` Chris Down
2021-02-18 12:37   ` Petr Mladek
2021-02-18 12:41     ` Chris Down
2021-02-18 14:25       ` Petr Mladek
2021-02-18 15:53         ` Chris Down

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YCv4V5EFeuEmyxSz@chrisdown.name \
    --to=chris@chrisdown.name \
    --cc=akpm@linux-foundation.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hannes@cmpxchg.org \
    --cc=john.ogness@linutronix.de \
    --cc=keescook@chromium.org \
    --cc=kernel-team@fb.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).