From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Chris Down <chris@chrisdown.name>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Petr Mladek <pmladek@suse.com>, Jessica Yu <jeyu@kernel.org>,
Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
John Ogness <john.ogness@linutronix.de>,
Steven Rostedt <rostedt@goodmis.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Johannes Weiner <hannes@cmpxchg.org>,
Kees Cook <keescook@chromium.org>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
kernel-team@fb.com
Subject: Re: [PATCH v7 1/5] string_helpers: Escape double quotes in escape_special
Date: Wed, 16 Jun 2021 00:42:27 +0300 [thread overview]
Message-ID: <CAHp75Vc-edpD5kz0EPsO7Q=zOPHWFckZzc17imO85dQy-PpOgg@mail.gmail.com> (raw)
In-Reply-To: <af144c5b75e41ce417386253ba2694456bc04118.1623775748.git.chris@chrisdown.name>
On Tue, Jun 15, 2021 at 7:53 PM Chris Down <chris@chrisdown.name> wrote:
In case you will need to send a new version some nit picks below WRT
commit message. Or somebody might fix them in place when applying.
> From an abstract point of view, escape_special's counterpart,
> unescape_special, already handles the unescaping of blackslashed double
> quote sequences.
>
> As a more practical example, printk indexing is an example case where
(example example)
"As a more practical example, printk indexing is the case where..."
?
> this is already practically useful. Compare an example with
> `ESCAPE_SPECIAL | ESCAPE_SPACE`, with quotes not escaped:
>
> [root@ktst ~]# grep drivers/pci/pci-stub.c:69 /sys/kernel/debug/printk/index/vmlinux
> <4> drivers/pci/pci-stub.c:69 pci_stub_init "pci-stub: invalid ID string "%s"\n"
>
> ...and the same after this patch:
>
> [root@ktst ~]# grep drivers/pci/pci-stub.c:69 /sys/kernel/debug/printk/index/vmlinux
> <4> drivers/pci/pci-stub.c:69 pci_stub_init "pci-stub: invalid ID string \"%s\"\n"
In both examples: '[root@ktst ~]#' => '#'
> One can of course, alternatively, use ESCAPE_APPEND with a quote in
> @only, but without this patch quotes are coerced into hex or octal which
> can hurt readability quite significantly.
>
> I've checked uses of ESCAPE_SPECIAL and %pE across the codebase, and I'm
checked the uses
> pretty confident that this shouldn't affect any stable interfaces.
>
> Signed-off-by: Chris Down <chris@chrisdown.name>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> lib/string_helpers.c | 4 ++++
> lib/test-string_helpers.c | 14 +++++++-------
> 2 files changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 5a35c7e16e96..3806a52ce697 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -361,6 +361,9 @@ static bool escape_special(unsigned char c, char **dst, char *end)
> case '\e':
> to = 'e';
> break;
> + case '"':
> + to = '"';
> + break;
> default:
> return false;
> }
> @@ -474,6 +477,7 @@ static bool escape_hex(unsigned char c, char **dst, char *end)
> * '\t' - horizontal tab
> * '\v' - vertical tab
> * %ESCAPE_SPECIAL:
> + * '\"' - double quote
> * '\\' - backslash
> * '\a' - alert (BEL)
> * '\e' - escape
> diff --git a/lib/test-string_helpers.c b/lib/test-string_helpers.c
> index 2185d71704f0..437d8e6b7cb1 100644
> --- a/lib/test-string_helpers.c
> +++ b/lib/test-string_helpers.c
> @@ -140,13 +140,13 @@ static const struct test_string_2 escape0[] __initconst = {{
> },{
> .in = "\\h\\\"\a\e\\",
> .s1 = {{
> - .out = "\\\\h\\\\\"\\a\\e\\\\",
> + .out = "\\\\h\\\\\\\"\\a\\e\\\\",
> .flags = ESCAPE_SPECIAL,
> },{
> - .out = "\\\\\\150\\\\\\042\\a\\e\\\\",
> + .out = "\\\\\\150\\\\\\\"\\a\\e\\\\",
> .flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
> },{
> - .out = "\\\\\\x68\\\\\\x22\\a\\e\\\\",
> + .out = "\\\\\\x68\\\\\\\"\\a\\e\\\\",
> .flags = ESCAPE_SPECIAL | ESCAPE_HEX,
> },{
> /* terminator */
> @@ -157,10 +157,10 @@ static const struct test_string_2 escape0[] __initconst = {{
> .out = "\eb \\C\007\"\x90\\r]",
> .flags = ESCAPE_SPACE,
> },{
> - .out = "\\eb \\\\C\\a\"\x90\r]",
> + .out = "\\eb \\\\C\\a\\\"\x90\r]",
> .flags = ESCAPE_SPECIAL,
> },{
> - .out = "\\eb \\\\C\\a\"\x90\\r]",
> + .out = "\\eb \\\\C\\a\\\"\x90\\r]",
> .flags = ESCAPE_SPACE | ESCAPE_SPECIAL,
> },{
> .out = "\\033\\142\\040\\134\\103\\007\\042\\220\\015\\135",
> @@ -169,10 +169,10 @@ static const struct test_string_2 escape0[] __initconst = {{
> .out = "\\033\\142\\040\\134\\103\\007\\042\\220\\r\\135",
> .flags = ESCAPE_SPACE | ESCAPE_OCTAL,
> },{
> - .out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\015\\135",
> + .out = "\\e\\142\\040\\\\\\103\\a\\\"\\220\\015\\135",
> .flags = ESCAPE_SPECIAL | ESCAPE_OCTAL,
> },{
> - .out = "\\e\\142\\040\\\\\\103\\a\\042\\220\\r\\135",
> + .out = "\\e\\142\\040\\\\\\103\\a\\\"\\220\\r\\135",
> .flags = ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_OCTAL,
> },{
> .out = "\eb \\C\007\"\x90\r]",
> --
> 2.31.1
>
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2021-06-15 21:42 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-15 16:52 [PATCH v7 0/5] printk: Userspace format indexing support Chris Down
2021-06-15 16:52 ` [PATCH v7 1/5] string_helpers: Escape double quotes in escape_special Chris Down
2021-06-15 21:42 ` Andy Shevchenko [this message]
2021-06-16 0:00 ` Chris Down
2021-06-16 2:05 ` Randy Dunlap
2021-06-16 8:08 ` Andy Shevchenko
2021-06-16 13:30 ` Steven Rostedt
2021-06-15 16:52 ` [PATCH v7 2/5] printk: Straighten out log_flags into printk_info_flags Chris Down
2021-06-15 16:52 ` [PATCH v7 3/5] printk: Rework parse_prefix into printk_parse_prefix Chris Down
2021-06-17 8:02 ` Petr Mladek
2021-06-15 16:52 ` [PATCH v7 4/5] printk: Userspace format indexing support Chris Down
2021-06-17 7:36 ` kernel test robot
2021-06-17 7:36 ` kernel test robot
2021-06-17 8:38 ` Andy Shevchenko
2021-06-17 8:38 ` Andy Shevchenko
2021-06-17 9:39 ` Petr Mladek
2021-06-17 9:39 ` Petr Mladek
2021-06-17 11:57 ` Chris Down
2021-06-17 11:57 ` Chris Down
2021-06-17 9:43 ` Petr Mladek
2021-06-18 11:48 ` kernel test robot
2021-06-18 11:48 ` kernel test robot
2021-06-18 12:19 ` Chris Down
2021-06-18 12:19 ` Chris Down
2021-06-18 15:00 ` Petr Mladek
2021-06-18 15:00 ` Petr Mladek
2021-06-15 16:52 ` [PATCH v7 5/5] printk: index: Add indexing support to dev_printk Chris Down
2021-06-17 10:02 ` Petr Mladek
2021-06-17 10:10 ` [PATCH v7 0/5] printk: Userspace format indexing support Petr Mladek
2021-06-17 12:33 ` Andy Shevchenko
2021-06-17 14:50 ` Chris Down
2021-06-17 14:52 ` Chris Down
2021-06-17 12:36 ` Jessica Yu
2021-06-21 8:54 ` Petr Mladek
2021-06-23 21:08 ` Andrew Morton
2021-06-25 9:35 ` Petr Mladek
2021-07-19 13:06 ` Petr Mladek
2021-07-19 13:08 ` Petr Mladek
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='CAHp75Vc-edpD5kz0EPsO7Q=zOPHWFckZzc17imO85dQy-PpOgg@mail.gmail.com' \
--to=andy.shevchenko@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=chris@chrisdown.name \
--cc=gregkh@linuxfoundation.org \
--cc=hannes@cmpxchg.org \
--cc=jeyu@kernel.org \
--cc=john.ogness@linutronix.de \
--cc=keescook@chromium.org \
--cc=kernel-team@fb.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.