Re: [PATCH]tracepoint.c: Add conditionals num to find_matching_tracepoint

[Hui Zhu <teawater@gmail.com>]

Ping.

Thanks,
Hui

On Sun, Aug 14, 2011 at 21:40, Hui Zhu <teawater@gmail.com> wrote:
> On Tue, Aug 9, 2011 at 07:02, Stan Shebs <stanshebs@earthlink.net> wrote:
>> On 8/7/11 9:28 AM, Hui Zhu wrote:
>>>
>>> Hi,
>>>
>>> I found that when I set some tracepoint to a same address, and use
>>> tsave.   And use "target tfile " to open it.  It will only one
>>> tracepoint available.
>>> And output some words like:
>>> Created tracepoint 1 for target's tracepoint 1 at 0x40050a.
>>> Assuming tracepoint 1 is same as target's tracepoint 2 at 0x40050a.
>>> Assuming tracepoint 1 is same as target's tracepoint 3 at 0x40050a.
>>>
>>> This is because find_matching_tracepoint didn't check the num.
>>
>>
>> The number is exactly the one property that tracepoint upload must never
>> consider when looking for matching tracepoints, since the numbers vary
>> depending on what the user has been doing during the current GDB session.
>>  Addressing the FIXME will help the multiple-tracepoint case, although it's
>> kind of messy.
>>
>> There is another useful heuristic that would be easy to add, which is to
>> exclude matching on a tracepoint that has already been uploaded.  Having
>> created tracepoint 1 from the uploaded info, it's never going to be the case
>> that uploaded tracepoints 2 and 3 are the same as 1.  It might be as simple
>> as testing number_on_target, but beware that it might be nonzero due to
>> other trace runs or some such.
>>
>> Stan
>> stan@codesourcery.com
>>
>>>
>>> And I think the tracepoint have the same address is really helpful for
>>> user.  Because we can set different condition to make tracepoint more
>>> powerful.
>>> So I make following patch.
>>>
>>> Please help me review it.
>>>
>>> Thanks,
>>> Hui
>>>
>>>
>>> 2011-08-08  Hui Zhu<teawater@gmail.com>
>>>
>>>        * tracepoint.c (find_matching_tracepoint): Add number check.
>>
>>
>
> Hi Stan,
>
> Thanks for your review.
>
> I make a new patch that check the condition according to your mail.
>
> Best,
> Hui
>
> 2011-08-14  Hui Zhu  <teawater@gmail.com>
>
>        * tracepoint.c (cond_string_is_same): New function.
>        (find_matching_tracepoint): Add condition check
>        by cond_string_is_same.
> ---
>  tracepoint.c |   19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> --- a/tracepoint.c
> +++ b/tracepoint.c
> @@ -3091,6 +3091,22 @@ free_uploaded_tsvs (struct uploaded_tsv
>     }
>  }
>
> +static int
> +cond_string_is_same(char *str1, char *str2)
> +{
> +  if (str1 == NULL || str2 == NULL)
> +    {
> +      if (str1 == str2)
> +       return 1;
> +      else
> +       return 0;
> +    }
> +  if (strcmp (str1, str2))
> +    return 0;
> +
> +  return 1;
> +}
> +
>  /* Look for an existing tracepoint that seems similar enough to the
>    uploaded one.  Enablement isn't compared, because the user can
>    toggle that freely, and may have done so in anticipation of the
> @@ -3111,7 +3127,8 @@ find_matching_tracepoint (struct uploade
>       if (b->type == utp->type
>          && t->step_count == utp->step
>          && t->pass_count == utp->pass
> -         /* FIXME also test conditionals and actions.  */
> +         && cond_string_is_same (t->base.cond_string, utp->cond_string)
> +         /* FIXME also test actions.  */
>          )
>        {
>          /* Scan the locations for an address match.  */
>