Re: [PATCH] Some tracepoint fixes

[Daniel Jacobowitz <drow@false.org>]

On Wed, Feb 09, 2005 at 09:19:17AM +0000, Nathan Sidwell wrote:
> In porting gdb to a new architecture, I came across a number of core gdb
> bugs.   Here is the first set of them and addresses the following issues,
> 
> 1) we did not allow 'extended-remote' targets to use tracepoints.
> 
> 2) We could only trace architectures with 64 registers, not 256 like
> a comment suggested.
> 
> 3) There was an erroneous comment about tracing memory ranges
> 
> 4) If a ^D was entered when entering the 'actions' list, we'd create
> a NULL action, which would cause a segfault when tracing started.
> 
> 5) The 'tstatus' command did not actually print any status. testcase
> gdb.trace/tfind.exp exepected it to do so.
> 
> 6) Parsing the tfind responses uses strtol to read hex.  That reads
> 'FFFFFFFF' as '7FFFFFFF' (and sets errno).  Using sscanf reads that as
> -1, as desired.
> 
> built and tested on i686-pc-linux-gnu, (and on the unreleased architecture
> I ported to) ok?

Hi Nathan,

Some bits of this are OK, some aren't (and I'd want Michael's opinion
on them).  In particular, the tracepoint remote protocol packets don't
appear to be documented; so I'm not sure about the strtol change.  Your
change is definitely wrong one way or another, because it depends on
the size of "long" on the host.  You're passing a long* to sscanf where
it expects an unsigned long*.  If we expect a hex-encoded 32-bit
result, then let's parse it that way explicitly.

> *************** trace_status_command (char *args, int fr
> *** 1873,1878 ****
> --- 1877,1886 ----
>   
>         /* exported for use by the GUI */
>         trace_running_p = (target_buf[1] == '1');
> +       if (trace_running_p)
> + 	printf_filtered ("Trace is running.\n");
> +       else
> + 	printf_filtered ("Trace is not running.\n");
>       }
>     else
>       error ("Trace can only be run on remote targets.");

That's really weird.  This hasn't been there as far back as the code
has been in the FSF repository, but the testsuite definitely tests for
it... it's pretty useless in the CLI as it is.  Michael, any idea?

-- 
Daniel Jacobowitz
CodeSourcery, LLC