Re: [patch 4/8] Download tracepoint locations and track its status

[Pedro Alves <pedro@codesourcery.com>]

On Tuesday 08 November 2011 06:30:17, Yao Qi wrote:

> @@ -10660,6 +10713,8 @@ update_global_location_list (int should_insert)
>           || (gdbarch_has_global_breakpoints (target_gdbarch))))
>      insert_breakpoint_locations ();
>  
> +  download_tracepoint_locations ();

Should be guarded by `should_insert'.

> -      /* Skip LOCP entries which will definitely never be needed.
> +      /* skip LOCP entries which will definitely never be needed.

Drop this spurious change please.

> @@ -10491,7 +10539,9 @@ update_global_location_list (int should_insert)
>                              if it should be inserted in case it will be
>                              unduplicated.  */
>                           if (loc2 != old_loc
> -                             && unduplicated_should_be_inserted (loc2))
> +                             && unduplicated_should_be_inserted (loc2)
> +                             && (is_tracepoint (old_loc->owner)
> +                                 == is_tracepoint (loc2->owner)))

What if both are tracepoints, but of different kind?  I think this
might be better handled within breakpoint_locations_match (called just above).


>    /* Nonzero if this is not the first breakpoint in the list
> -     for the given address.  */
> +     for the given address.  In current implementation, location
> +     of tracepoint never be duplicated with other locations of
> +     tracepoints and other kinds of breakpoints, because two locations
> +     at the same address may have different actions, so both of
> +     these locations should be downloaded. 

and so that "tfind N" always works.

Please drop the "In current implementation" bit, and the "it is
possible" comment.

> +  /* Locations of tracepoints never be duplicated.  */

A verb is missing.  "can never be", perhaps?

> +  if (is_tracepoint (left->owner))
> +    gdb_assert (left->duplicate == 0);

`duplicate' is a boolean.  Make that `gdb_assert (!left->duplicate)'.

> +  if (is_tracepoint (right->owner))
> +    gdb_assert (right->duplicate == 0);

> +         gdb_assert (loc->inserted == 0);

Boolean ==> `gdb_assert (!loc->inserted)'

-- 
Pedro Alves