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

[Hui Zhu <teawater@gmail.com>]

Hi Pedro,

Thanks for your review.

On Tue, Apr 9, 2013 at 8:03 PM, Pedro Alves <palves@redhat.com> wrote:
> Hi Hui,
>
> On 04/09/2013 10:00 AM, Hui Zhu wrote:
>> On Mon, Apr 8, 2013 at 10:39 PM, Pedro Alves <palves@redhat.com> wrote:
>>>
>>>> +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.
>
> Close, but:
>
>> +standard_testfile .c
>> +set libfile "dprintf-pendshr"
>> +set libsrc  $srcdir/$subdir/$libfile.c
>> +set lib_sl  $objdir/$subdir/$libfile.sl
>
> ".c" is not necessary.  Do use standard_output_file please.
>
> Like so:
>
> standard_testfile
>
> set libfile "dprintf-pendshr"
> set libsrc  $srcdir/$subdir/$libfile.c
> set lib_sl [standard_output_file $libfile.sl]

Fixed.

>
>
> Oh,
>
>> +++ b/gdb/breakpoint.c
>> @@ -12963,6 +12963,35 @@ bkpt_re_set (struct breakpoint *b)
>>    breakpoint_re_set_default (b);
>>  }
>>
>> +/* Dprintf breakpoint_ops methods.  */
>> +
>> +static void
>> +dprintf_re_set (struct breakpoint *b)
>> +{
> ...
>> +}
>> +
>>  static int
>>  bkpt_insert_location (struct bp_location *bl)
>>  {
>
> I only now noticed you're putting the new dprintf_re_set
> function right in the middle of the
>
> /* Default breakpoint_ops methods.  */
>
> bkpt_re_set (struct breakpoint *b)
> ...
> bkpt_insert_location (struct bp_location *bl)
> ...
> bkpt_remove_location (struct bp_location *bl)
> ...
> bkpt_breakpoint_hit (const struct bp_location *bl,
> ...
> bkpt_resources_needed (const struct bp_location *bl)
> ...
> bkpt_print_it (bpstat bs)
> ...
> bkpt_print_mention (struct breakpoint *b)
> ...
> bkpt_decode_linespec (struct breakpoint *b, char **s,
> ...
>
> section.  If you keep scrolling down, you'll notice that
> we have a section/block/group of functions for each
> breakpoint type.
>
> /* Virtual table for internal breakpoints.  */
> /* Virtual table for momentary breakpoints  */
> /* Specific methods for probe breakpoints.  */
> /* The breakpoint_ops structure to be used in tracepoints.  */
>
> etc. for all other types.  Please don't break this layout.

Fixed.

>
> Otherwise OK.

Commited in http://sourceware.org/ml/gdb-cvs/2013-04/msg00097.html

Thanks,
Hui

>
> Thanks,
> --
> Pedro Alves
>