Re: [PATCH] add -s option to make -break-insert support dprintf

[Hui Zhu <teawater@gmail.com>]

On Tue, Apr 16, 2013 at 12:05 AM, Pedro Alves <palves@redhat.com> wrote:
> On 04/15/2013 02:33 PM, Hui Zhu wrote:
>> On Sat, Apr 13, 2013 at 12:32 AM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>>> "Hui" == Hui Zhu <teawater@gmail.com> writes:
>>>
>>> Hui> +    [ -s "@var{template},@var{expression}[,@var{expression}@dots{}]" ]
>>> Hui> +    [ @var{location} ]
>>>
>>> Hui> +@item -s "@var{template},@var{expression}[,@var{expression}@dots{}]"
>>> Hui> +Set a dynamic printf breakpoint, described in @ref{Dynamic Printf}.
>>> Hui> +The @var{template} and @var{expression} should be within double
>>> Hui> +quotes and be escaped by being preceded with a backslash.
>>>
>>> MI already defines a quoting approach and allows multiple arguments.
>>> In fact one of its selling points is that it doesn't have to be as
>>> free-form as the CLI -- it can be more predictable for programs to use.
>>>
>>> So, I think the above approach is not that great.
>>> It adds a second layer of parsing to MI, I guess just to work around
>>> internal deficiencies in gdb.
>>>
>>> It seems like you could use positional arguments instead:
>>>
>>> -break-insert -s FORMAT LOCATION ARG ARG ARG ...
>>>
>>>
>>> I don't really understand the part about how the expressions should be
>>> in double quotes.  The test suite doesn't do that:
>>>
>>>     +mi_gdb_test "5-break-insert -s \"\\\"arg=%d, g=%d\\\\n\\\", arg, g\" $dp_location1" \
>>>
>>> I suggest just dropping that text.  I think it probably means that the
>>> argument should be properly quoted for MI, but that is redundant.
>
> I stopped for this as well before while reviewing the patches.  I ended
> up not say anything as after trying things out, I realized that unlike
> with other arguments, which only require quoting if the argument itself
> contains spaces or quotes, etc., in this case, the quotes really are
> necessary (as in, GDB complains if you don't add the quotes).  I didn't
> think about the split arguments idea though.  That's a good question.
>
>>>
>>> Tom
>>
>> This design is because the MI inferior will auto convert the format
>> string of argument of mi command.  But dprintf need format sting keep
>> the original of the format string.  So I use the current format.
>
> I have a bit of trouble understanding exactly what you meant.  :-(  I see
> that with "target-printf" (what dprintf uses as command when the dprintf
> runs on the target/agent side), the whole format string plus the expressions are
> passed down to the target as a single chunk, unanalyzed and unmodified.
> E.g., even 'dprintf foo, "foo" "bar"' or 'agent-printf "foo" "bar"' is
> accepted, and passed down to the target as is (whatever that might mean).
> Is that what you were referring to?  What did you mean by "MI inferior"
> and "auto convert" ?

The MI handle the argument is different with CLI.  For example:
Input -break-insert -s "1\n" to mi.  The mi_cmd_break_insert will get:
Breakpoint 1, mi_cmd_break_insert (command=0x233fb50 "break-insert",
argv=0x233fd30, argc=2)
    at ../../gdb/gdb/mi/mi-cmd-break.c:93
93  char *address = NULL;
(gdb) p argv[0]
$1 = 0x233fd50 "-s"
(gdb) p argv[1]
$2 = 0x233fd70 "1\n"
(gdb) p argv[1][0]
$3 = 49 '1'
(gdb) p argv[1][1]
$4 = 10 '\n'

But the dprint want to get is:
(gdb) p argv[1][1]
$7 = 49 '1'
(gdb) p argv[1][2]
$8 = 92 '\\'
(gdb) p argv[1][3]
$9 = 110 'n'

Thanks,
Hui


>
> --
> Pedro Alves
>