[re:,v2,00/15,-,03/11] dyndbg: disambiguate quoting in a debug msg

  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.

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: <*>

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

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.
> 
> 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.

Also could you please provide output before and after?

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.

I do not understand how this would help. The double quote is gone
even in this variant.

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

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".

Best Regards,
Petr
  

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

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" >
/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

>
> 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"

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)

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"

extraneous unnecessary quotes are one thing to take away.
then the remaining quotes are doing something,
like preventing shell expansion, protecting inner quoting, etc

OTOH, the single whole-query arg, in single quotes has its own clarity.
Almost all examples are preserved - only 1 was changed to quote-less form

and seeing <> in legitimate input should be exceptionally rare, so
pretty unambiguous as balanced quotes

#> echo module dunno format ' "looking for <foo> in format" ' +p >
/proc/dynamic_debug/control
[1400065.833314] dyndbg: read 53 bytes from userspace
[1400065.833324] dyndbg: query 0: "module dunno format "looking for
<foo> in format" +p" mod:*
[1400065.833331] dyndbg: split into words: "module" "dunno" "format"
"looking for <foo> in format" "+p"
[1400065.833345] dyndbg: op='+'
[1400065.833348] dyndbg: flags=0x1
[1400065.833351] dyndbg: *flagsp=0x1 *maskp=0xffffffff
[1400065.833356] dyndbg: parsed: func="" file="" module="dunno"
format="looking for <foo> in format" lineno=0-0 class=(null)
[1400065.833403] dyndbg: no matches for query
[1400065.833406] dyndbg: no-match: func="" file="" module="dunno"
format="looking for <foo> in format" lineno=0-0 class=(null)
[1400065.833412] dyndbg: processed 1 queries, with 0 matches, 0 errs

new form (w combined op-flags-mask-dest patch too)

[root@v6 lx]# echo 'module dunno format "looking for <foo> in format"'
+p > /proc/dynamic_debug/control
[  737.645791] dyndbg: read 53 bytes from userspace
[  737.646374] dyndbg: query 0: <module dunno format "looking for
<foo> in format" +p> on module: <*>
[  737.647226] dyndbg: split into words: "module" "dunno" "format"
"looking for <foo> in format" "+p"
[  737.648069] dyndbg: op='+' flags=0x1 maskp=0xffffffff trace_dest=0x0
[  737.648673] dyndbg: no matches for query
[  737.649162] dyndbg: no-match: func="" file="" module="dunno"
format="looking for <foo> in format" lineno=0-0 class=(null)
[  737.650481] dyndbg: processed 1 queries, with 0 matches, 0 errs


>
> Best Regards,
> Petr

Am I materially closer to what youd want to see?
thx, Jim
  

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
  

> I still thing that using "echo" in the form of
>
>      $> echo param param param
>
> is uncommon and even misleading.
>
> Best Regards,
> Petr

we can differ on this.

as to the patch itself,
you dont like it,
I dont need it,
we'll drop it and I'll fix the following commit msg