Message-ID: <ZZV_2hTP1VQB3VZm@alley>
Date: Thu, 4 Jan 2024 08:54:12 +0100
From: Petr Mladek <pmladek@...e.com>
To: jim.cromie@...il.com
Cc: lb@...ihalf.com, linux-kernel@...r.kernel.org,
	akpm@...ux-foundation.org, bleung@...gle.com, contact@...rsion.fr,
	daniel@...ll.ch, dianders@...omium.org, groeck@...gle.com,
	jbaron@...mai.com, john.ogness@...utronix.de, keescook@...omium.org,
	ppaalanen@...il.com, rostedt@...dmis.org, seanpaul@...omium.org,
	sergey.senozhatsky@...il.com, upstream@...ihalf.com,
	vincent.whitchurch@...s.com, yanivt@...gle.com,
	gregkh@...uxfoundation.org
Subject: Re: [re: PATCH v2 00/15 - 03/11] dyndbg: disambiguate quoting in a
 debug msg

On Tue 2023-12-19 16:38:31, jim.cromie@...il.com wrote:
> On Mon, Dec 18, 2023 at 9:34 AM Petr Mladek <pmladek@...e.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@...do wk-test]# echo module '"$modname"' func '*' "$flagmods" >

and this one uses '"$modname"'

> /proc/dynamic_debug/control
> bash: echo: write error: Invalid argument
> [root@...do 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@...ail.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

Powered by blists - more mailing lists