On 02/25/2013 02:57 AM, Yao Qi wrote:
> The previous patch exposes a bug in tfile target when finding a trace
> frame.  Every time, GDB will scan tfile from the starting offset and
> initialize trace frame number to zero.  When TYPE is not tfind_number,
> it means GDB wants to find the *next* matched trace frame of current
> one.  So we need to check the tfile trace frame number iterator is
> greater than the current trace frame number (which means *next*).
> This is mainly what this patch does.

Thanks.  Good catch.

> diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
> index ca104aa..f7a3650 100644
> --- a/gdb/tracepoint.c
> +++ b/gdb/tracepoint.c
> @@ -4324,22 +4324,34 @@ tfile_trace_find (enum trace_find_type type, int num,
>  	  break;
>  	case tfind_pc:
>  	  tfaddr = tfile_get_traceframe_address (tframe_offset);
> -	  if (tfaddr == addr1)
> +	  if (tfaddr == addr1
> +	      /* Looks for the next trace frame if matched.  */
> +	      && (tfnum > traceframe_number
> +		  || (tfnum == traceframe_number && tfnum == 0)))
>  	    found = 1;
>  	  break;
>  	case tfind_tp:
>  	  tp = get_tracepoint (num);
> -	  if (tp && tpnum == tp->number_on_target)
> +	  if (tp && tpnum == tp->number_on_target
> +	      /* Looks for the next trace frame if matched.  */
> +	      && (tfnum > traceframe_number
> +		  || (tfnum == traceframe_number && tfnum == 0)))
>  	    found = 1;
>  	  break;
>  	case tfind_range:
>  	  tfaddr = tfile_get_traceframe_address (tframe_offset);
> -	  if (addr1 <= tfaddr && tfaddr <= addr2)
> +	  if (addr1 <= tfaddr && tfaddr <= addr2
> +	      /* Looks for the next trace frame if matched.  */
> +	      && (tfnum > traceframe_number
> +		  || (tfnum == traceframe_number && tfnum == 0)))
>  	    found = 1;
>  	  break;
>  	case tfind_outside:
>  	  tfaddr = tfile_get_traceframe_address (tframe_offset);
> -	  if (!(addr1 <= tfaddr && tfaddr <= addr2))
> +	  if (!(addr1 <= tfaddr && tfaddr <= addr2)
> +	      /* Looks for the next trace frame if matched.  */
> +	      && (tfnum > traceframe_number
> +		  || (tfnum == traceframe_number && tfnum == 0)))

I'm confused on why this bit of the predicate

 (tfnum == traceframe_number && tfnum == 0)

is necessary.  traceframe_number is -1 when not looking
at a traceframe yet, so "tfnum > traceframe_number"
should be sufficient, no?

I find it clearer to move the frame skipping a bit higher
up, even before the specific tfind tp/range/etc. matching.
Doing it this way also avoids unnecessary read/lseek system
calls done by tfile_get_traceframe_address.

WDYT?