This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch 4/8] Download tracepoint locations and track its status
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