mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: jim.cromie@gmail.com
Cc: lb@semihalf.com, linux-kernel@vger.kernel.org,
	akpm@linux-foundation.org, bleung@google.com,
	contact@emersion.fr, daniel@ffwll.ch, dianders@chromium.org,
	groeck@google.com, jbaron@akamai.com, john.ogness@linutronix.de,
	keescook@chromium.org, ppaalanen@gmail.com, rostedt@goodmis.org,
	seanpaul@chromium.org, sergey.senozhatsky@gmail.com,
	upstream@semihalf.com, vincent.whitchurch@axis.com,
	yanivt@google.com, gregkh@linuxfoundation.org
Subject: Re: [re: PATCH v2 00/15 - 03/11] dyndbg: disambiguate quoting in a debug msg
Date: Thu, 4 Jan 2024 08:54:12 +0100	[thread overview]
Message-ID: <ZZV_2hTP1VQB3VZm@alley> (raw)
In-Reply-To: <CAJfuBxyOpPfks69LKOfiz7TPeSmqhCvHzv2qrCwn2CXfZLHJFA@mail.gmail.com>

On Tue 2023-12-19 16:38:31, jim.cromie@gmail.com wrote:
> On Mon, Dec 18, 2023 at 9:34 AM Petr Mladek <pmladek@suse.com> wrote:
> >
> > On Thu 2023-12-07 17:15:06, Jim Cromie wrote:
> > > When debugging a query parsing error, the debug message wraps the
> > > query in escaped-double-quotes.  This is confusing when mixed with any
> > > quoted args where quotes are stripped by the shell.
> 
> (with dynamic_debug.verbose=3)
> nobody will be looking at this unless their query doesnt work.
> 
> > > So this replaces the \"%s\" with <%s> in the format string, allowing a
> > > user to see how the shell strips quotes:
> > >
> > > lx]# echo  module "foo" format ,_ -f > /proc/dynamic_debug/control
> > > [  716.037430] dyndbg: read 26 bytes from userspace
> > > [  716.037966] dyndbg: query 0: <module foo format ,_ -f> on module: <*>
> >
> > Could you provide a real life example, please? It is hard to imagine
> > what '"foo" format' means in a real life.
> 
> yes, sorry.  that was a poor selection from a bunch of output:
>      cat control-fuzz-cmds > /proc/dynamic_debug/control
> 
> that said, it was well formed input:  <module "foo" format ,_ -f>
> 
> >
> > Also could you please provide output before and after?
> 
> will do.
> 
> > Honestly, Using <> as quotes looks pretty non-standard and confusing
> > to me. Also this changes only one place but '\"' is used in many
> > other locations which would make dyndbg messages even more confusing.
> 
> perhaps I was myopic.
> 
> >
> > I do not understand how this would help. The double quote is gone
> > even in this variant.
> >
> 
> let me find a more compelling example.
> If I dont, maybe I'll drop (or shelve) this, I dont need it anymore.
> 
> > BTW: It is a bit funny that this patch is supposed to make the debug
> >      message better readable. For me, the echo command is hard
> >      to read in the first place. I would use:
> >
> > lx]# echo "module $my_module ,_ -f" > /proc/dynamic_debug/control
> 
> someone doing it in a script might want to control / quote $vars more actively:
> 
>    echo module "$modname" func '*' "$flagmods" >  /proc/dynamic_debug/control

This example uses: "$modname"

> if those vars arent set, it errs like this:
> 
> [root@v6 lx]# vx 3 # verbose=3
> [root@v6 lx]# echo module ' "$modname" ' func '*' "$flagmods" >
> /proc/dynamic_debug/control
> [ 3114.654016] dyndbg: read 26 bytes from userspace
> [ 3114.654314] dyndbg: query 0: <module "$modname" func * > on module: <*>
> [ 3114.654759] dyndbg: split into words: "module" "$modname" "func" "*"
> [ 3114.655319] dyndbg: expecting pairs of match-spec <value>
> [ 3114.655714] dyndbg: selector parse failed  # s/selector/filters/
> [ 3114.655981] dyndbg: processed 1 queries, with 0 matches, 1 errs
> bash: echo: write error: Invalid argument
> 
> or in old form, like this:
> 
> [root@frodo wk-test]# echo module '"$modname"' func '*' "$flagmods" >

and this one uses '"$modname"'

> /proc/dynamic_debug/control
> bash: echo: write error: Invalid argument
> [root@frodo wk-test]# [1387800.269898] dyndbg: read 26 bytes from userspace
> [1387800.269902] dyndbg: query 0: "module "$modname" func * " mod:*
> [1387800.269904] dyndbg: split into words: "module" "$modname" "func" "*"
> [1387800.269909] dyndbg: bad flag-op *, at start of *
> [1387800.269911] dyndbg: flags parse failed
> [1387800.269912] dyndbg: processed 1 queries, with 0 matches, 1 errs
> 
> in that query 0, theres a lot of double-quotes, not quite looking right.
> the following split-line adds its own quotes, which might clarify, or not,
> but is verbose=3, where others are verbose=2 or 1

The string "$modname" is here only because of the outer single quotes ''.
Otherwise, $modname would be substituted to the value here.

IMHO, this example does not look realistic.

Or if you think that it is realistic then a better solution would be
to print either:

	'module "$modname" func * '

or

	"module \"$modname\" func \* "

because the already substituted string is written to
/proc/dynamic_debug/control.

> >
> > Maybe, this change fixes the output to match some personal style.
> > I wonder how common is the style. I can't remember seeing:
> >
> >     $> echo param param param
> >
> > Instead I frequently see:
> >
> >     $> echo "bla bla bla".
> 
> I suppose you could call it taste / personal preference.
> I did document the bareword form to the howto,
> but the form was always accepted input.
> It is the point of the 1st paragraph in the "Command Language Reference"

Yes, both variants work because "echo" would write all parameters
on the same line. The only difference is how the white space characters
are handled:

   echo bla    bla    bla   # would print: bla bla bla
while
   echo "bla    bla    bla"   # would print: bla    bla    bla

> commit ace7c4bbb240d076a9e2079027252420d920d0d0
> Author: Jim Cromie <jim.cromie@7gmail.com>
> Date:   Sun Sep 4 15:40:56 2022 -0600
> 
>     doc-dyndbg: edit dynamic-debug-howto for brevity, audience
>     Rework/modernize docs:
>    (trimmed)
>      - alias ddcmd='echo $* > /proc/dynamic_debug/control
>        focus on args: declutter, hide boilerplate, make pwd independent.
>      - simplify - drop extra words, phrases, sentences.
> 
> but I only added 1 example, other single-arg examples are materially preserved
> (modulo the ddcmd usage, again for brevity)

Anyway, all the real life examples in Documentation/admin-guide/dynamic-debug-howto.rst
are using quotes, for example:

<paste>
  // enable the message at line 1603 of file svcsock.c
  :#> ddcmd 'file svcsock.c line 1603 +p'

  // enable all the messages in file svcsock.c
  :#> ddcmd 'file svcsock.c +p'

  // enable all the messages in the NFS server module
  :#> ddcmd 'module nfsd +p'
</paste>

:#> ddcmd 'file svcsock.c line 1603 +p'

> Antoine de Saint-Exupéry is credited with the quote,
> "Perfection is achieved, not when there is nothing more to add, but
> when there is nothing left to take away"

I do not remember in which context this was used. But I think
that this definition is not valid in all situations. For example,
a perfect home might be a prison cell from this POV. But only few
people would like to live there.

I still thing that using "echo" in the form of

     $> echo param param param

is uncommon and even misleading.

Best Regards,
Petr

  reply	other threads:[~2024-01-04  7:54 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-30 23:40 [PATCH v2 00/15] dyndbg: add support for writing debug logs to trace Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 01/15] dyndbg: add _DPRINTK_FLAGS_ENABLED Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 02/15] dyndbg: add _DPRINTK_FLAGS_TRACE Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 03/15] dyndbg: add write events to tracefs code Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 04/15] dyndbg: add 2 trace-events: prdbg, devdbg Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 05/15] tracefs: add __get_str_strip_nl - RFC Łukasz Bartosik
2023-12-01  0:25   ` Steven Rostedt
2023-12-01  9:00     ` Łukasz Bartosik
2023-12-08  0:15       ` [re: PATCH v2 00/15 - 00/11] dyndbg: add support for writing debug logs to trace Jim Cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 01/11] dyndbg: export _print_hex_dump Jim Cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 02/11] dyndbg: tweak pr_info format s/trace dest/trace_dest/ Jim Cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 03/11] dyndbg: disambiguate quoting in a debug msg Jim Cromie
2023-12-18 16:34           ` Petr Mladek
2023-12-19 23:38             ` jim.cromie
2024-01-04  7:54               ` Petr Mladek [this message]
2024-01-05  0:06                 ` jim.cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 04/11] dyndbg: fix old BUG_ON in >control parser Jim Cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 05/11] dyndbg: change +T:name_terminator to dot Jim Cromie
2023-12-18 17:29           ` Petr Mladek
2023-12-21 15:21             ` Łukasz Bartosik
2024-01-04  7:54               ` Petr Mladek
2024-01-08 12:21                 ` Łukasz Bartosik
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 06/11] dyndbg: treat comma as a token separator Jim Cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 07/11] dyndbg: __skip_spaces Jim Cromie
2023-12-18 17:17           ` Petr Mladek
2023-12-21 15:25             ` Łukasz Bartosik
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 08/11] dyndbg: split multi-query strings with % Jim Cromie
2023-12-09  0:32           ` Łukasz Bartosik
2023-12-11  1:07             ` jim.cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 09/11] dyndbg: reduce verbose/debug clutter Jim Cromie
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 10/11] dyndbg: move lock,unlock into ddebug_change, drop goto Jim Cromie
2023-12-09  0:32           ` Łukasz Bartosik
2023-12-08  0:15         ` [re: PATCH v2 00/15 - 11/11] dyndbg: id the bad word in parse-flags err msg Jim Cromie
2023-12-09  0:31         ` [re: PATCH v2 00/15 - 00/11] dyndbg: add support for writing debug logs to trace Łukasz Bartosik
2023-12-14 15:20           ` Łukasz Bartosik
2023-12-16  3:09             ` jim.cromie
2023-12-18  1:01               ` Łukasz Bartosik
2023-12-18 14:12                 ` Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 06/15] dyndbg: use __get_str_strip_nl in prdbg and devdbg Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 07/15] dyndbg: repack _ddebug structure Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 08/15] dyndbg: move flags field to a new structure Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 09/15] dyndbg: add trace destination field to _ddebug Łukasz Bartosik
2023-12-14  7:09   ` jim.cromie
2023-12-14 15:46     ` Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 10/15] dyndbg: add open and close commands for trace Łukasz Bartosik
2023-12-16  6:17   ` jim.cromie
2023-12-18  1:07     ` Łukasz Bartosik
2024-01-05 22:46   ` Jason Baron
2024-01-09 15:19     ` jim.cromie
2024-01-10 12:40       ` Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 11/15] dyndbg: don't close trace instance when in use Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 12/15] dyndbg: add processing of T(race) flag argument Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 13/15] dyndbg: add support for default trace destination Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 14/15] dyndbg: write debug logs to trace instance Łukasz Bartosik
2023-11-30 23:40 ` [PATCH v2 15/15] dyndbg: add support for hex_dump output to trace Łukasz Bartosik

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=ZZV_2hTP1VQB3VZm@alley \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=bleung@google.com \
    --cc=contact@emersion.fr \
    --cc=daniel@ffwll.ch \
    --cc=dianders@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@google.com \
    --cc=jbaron@akamai.com \
    --cc=jim.cromie@gmail.com \
    --cc=john.ogness@linutronix.de \
    --cc=keescook@chromium.org \
    --cc=lb@semihalf.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ppaalanen@gmail.com \
    --cc=rostedt@goodmis.org \
    --cc=seanpaul@chromium.org \
    --cc=sergey.senozhatsky@gmail.com \
    --cc=upstream@semihalf.com \
    --cc=vincent.whitchurch@axis.com \
    --cc=yanivt@google.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

all inboxes | Powered by JetHome®