Re: [PATCH 4/6] Notify breakpoint-modified when uploaded tracepoints are merged.

Pedro Alves
2012-12-06T20:57:26Z

On 12/04/2012 04:44 AM, Yao Qi wrote:
> When any location of a tracepoint is marked as installed/inserted,
> notify 'breakpoint-modified' observer.
>
> gdb:
>
> 2012-12-03 Yao Qi <yao@codesourcery.com>
>
> * tracepoint.c (merge_uploaded_tracepoints): Record all modified
> tracepoints and notify 'breakpoint-modified' observer for them.

This looks good to me. Some comment suggestions below.

I ran out of time for now, so I'll review the rest of the series later or tomorrow.

> ---
> gdb/tracepoint.c | 28 ++++++++++++++++++++++++++++
> 1 files changed, 28 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index c467b9c..0c023bd 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -3475,6 +3475,10 @@ void
> merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
> {
> struct uploaded_tp *utp;
> + /* A set of tracepoints which are modified. */
> + VEC(breakpoint_p) *modified_tp = NULL;
> + int ix;
> + struct breakpoint *b;
>
> /* Look for GDB tracepoints that match up with our uploaded versions. */
> for (utp = *uploaded_tps; utp; utp = utp->next)
> @@ -3485,6 +3489,8 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
> loc = find_matching_tracepoint_location (utp);
> if (loc)
> {
> + int found = 0;
> +
> /* Mark this location as already inserted. */
> loc->inserted = 1;
> t = (struct tracepoint *) loc->owner;
> @@ -3492,6 +3498,22 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
> "as target's tracepoint %d at %s.\n"),
> loc->owner->number, utp->number,
> paddress (loc->gdbarch, utp->addr));
> +
> + /* One location LOC of tracepoint is modified ('inserted' is
> + set to 1), save LOC->owner in MODIFIED_TP if LOC->owner
> + is not added to MODIFIED_TP before. The observer
> + 'breakpoint-modified' will be notified later with every
> + tracepoint saved in MODIFIED_TP. */

I suggest:

/* The tracepoint was modified (a location was marked as inserted
in the target). Save it in MODIFIED_TP if not there yet. The
'breakpoint-modified' observers will be notified later once for each
tracepoint saved in MODIFIED_TP. */

> + for (ix = 0;
> + VEC_iterate (breakpoint_p, modified_tp, ix, b);
> + ix++)
> + if (b == loc->owner)
> + {
> + found = 1;
> + break;
> + }
> + if (!found)
> + VEC_safe_push (breakpoint_p, modified_tp, loc->owner);
> }
> else
> {
> @@ -3514,6 +3536,12 @@ merge_uploaded_tracepoints (struct uploaded_tp **uploaded_tps)
> t->number_on_target = utp->number;
> }
>
> + /* Notify 'breakpoint-modified' observer that at least one of B's
> + locations is changed. */

/* Notify 'breakpoint-modified' observers that at least one of B's
locations was changed. */

> + for (ix = 0; VEC_iterate (breakpoint_p, modified_tp, ix, b); ix++)
> + observer_notify_breakpoint_modified (b);
> +
> + VEC_free (breakpoint_p, modified_tp);
> free_uploaded_tps (uploaded_tps);
> }

--
Pedro Alves