Message-ID: <CAJfuBxyOpPfks69LKOfiz7TPeSmqhCvHzv2qrCwn2CXfZLHJFA@mail.gmail.com>
Date: Tue, 19 Dec 2023 16:38:31 -0700
From: jim.cromie@...il.com
To: Petr Mladek <pmladek@...e.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 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

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

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

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

Powered by blists - more mailing lists