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

[Pedro Alves <palves@redhat.com>]

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]


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.

Otherwise OK.

Thanks,
-- 
Pedro Alves