Re: [PATCH] Fix dprintf work not right if it is pending

[Hui Zhu <teawater at gmail dot com>]

Hi Pedro,

Thanks for your review.

On Mon, Apr 8, 2013 at 10:39 PM, Pedro Alves <palves@redhat.com> wrote:
> Hi Hui,
>
> Getting there.
>
> On 04/08/2013 08:19 AM, Hui Zhu wrote:
>> +  /* 1 - connect to target 1, that can run breakpoint commands.
>> +     2 - create a dprintf, which resolves fine.
>> +     3 - disconnect from target 2
>
> This should have been "from target 1".

Fixed.

>
>> +     4 - connect to target 2, that can NOT run breakpoint commands.
>
>
>
>> +load_lib gdbserver-support.exp
>> +
>
> You're not calling gdbserver directly anymore, so this is no
> longer necessary.

Fixed.

>
>
>> +set testfile "dprintf-pending"
>> +set libfile "dprintf-pendshr"
>> +set srcfile $testfile.c
>> +set libsrc  $srcdir/$subdir/$libfile.c
>> +set binfile $objdir/$subdir/$testfile
>> +set lib_sl  $objdir/$subdir/$libfile.sl
>
> Please use standard_testfile/standard_output_file.

Fixed.

>
>
> Instead of these explicit "(without ...)":
>
>> +    "set pending dprintf (without format)" \
> ...
>> +    "set pending dprintf (without symbols)" \
> ...
>> +"single pending dprintf info (without symbols)"
> ...
>> +gdb_test "" ".*x=3.*x=4.*x=3.*" "run to resolved dprintf (without symbols)"
>
> Please use with_test_prefix, like
>
> with_test_prefix "without format" {
>   ... group all "without format" bits here, including test setup ...
> }
>
> with_test_prefix "without symbols" {
>   ... group all "without symbols" bits here ...
> }
>

Fixed.

>
>> +gdb_test "file ${binfile}" ".*Error in re-setting breakpoint.*" "load symbols get without format error"
>
> "load symbols get without format error" is not very correct English
> and I find it hard to grok.  I suggest instead "resolved dprintf fails to
> be re-set".  No need to mention "without format" here, as that will be
> covered by with_test_prefix.
>

Fixed.

> --
> Pedro Alves
>

Post a new version according to your comments.

Best,
Hui

2013-04-07  Pedro Alves  <palves@redhat.com>
   Hui Zhu  <hui@codesourcery.com>

* breakpoint.c (dprintf_re_set): New.
(initialize_breakpoint_ops): Set dprintf_breakpoint_ops re_set
to dprintf_re_set.

2013-04-07  Hui Zhu  <hui@codesourcery.com>

* gdb.base/Makefile.in (EXECUTABLES): Add dprintf-pending.
(MISCELLANEOUS): Add dprintf-pendshr.sl.
* gdb.base/dprintf-pending.c, gdb.base/dprintf-pending.exp: New.

Attachment: dprintf-pending.txt
Description: Text document

Attachment: dprintf-pending-test.txt
Description: Text document