Re: [PATCH] Tweaks in gdb.trace/tfile.exp

[Pedro Alves <palves at redhat dot com>]

On 08/21/2012 05:03 AM, Yao Qi wrote:

> 2012-08-21  Yao Qi  <yao@codesourcery.com>
> 
> 	* boards/native-gdbserver.exp (${board}_download): New.
> 	* boards/native-stdio-gdbserver.exp (${board}_download): New.

You mean ${board}_upload.

> 	* gdb.trace/tfile.c (write_basic_trace_file): Add one parameter.
> 	(write_error_trace_file): Likewise.
> 	Update caller.

callers.

> 	* gdb.trace/tfile.exp: Don't check 'gdb,nofileio'.
> 	Execute tfile on remote target.
> 	Copy trace file from target to host.
> ---
>  gdb/testsuite/boards/native-gdbserver.exp       |    4 ++++
>  gdb/testsuite/boards/native-stdio-gdbserver.exp |    4 ++++
>  gdb/testsuite/gdb.trace/tfile.c                 |   15 +++++++++------
>  gdb/testsuite/gdb.trace/tfile.exp               |   23 +++++++----------------
>  4 files changed, 24 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/testsuite/boards/native-gdbserver.exp b/gdb/testsuite/boards/native-gdbserver.exp
> index 89702df..7172274 100644
> --- a/gdb/testsuite/boards/native-gdbserver.exp
> +++ b/gdb/testsuite/boards/native-gdbserver.exp
> @@ -80,6 +80,10 @@ proc ${board}_download { board host dest } {
>      return $host
>  }
>  
> +proc ${board}_upload {dest srcfile args} {
> +    return $srcfile
> +}
> +
>  proc ${board}_file { dest op args } {
>      if { $op == "delete" } {
>  	return 0
> diff --git a/gdb/testsuite/boards/native-stdio-gdbserver.exp b/gdb/testsuite/boards/native-stdio-gdbserver.exp
> index 9804289..d8c5376 100644
> --- a/gdb/testsuite/boards/native-stdio-gdbserver.exp
> +++ b/gdb/testsuite/boards/native-stdio-gdbserver.exp
> @@ -142,6 +142,10 @@ proc ${board}_download { board host dest } {
>      return $host
>  }
>  
> +proc ${board}_upload {dest srcfile args} {
> +    return $srcfile
> +}
> +
>  proc ${board}_file { dest op args } {
>      if { $op == "delete" } {
>  	return 0
> diff --git a/gdb/testsuite/gdb.trace/tfile.c b/gdb/testsuite/gdb.trace/tfile.c
> index 11c79d9..5d58b26 100644
> --- a/gdb/testsuite/gdb.trace/tfile.c
> +++ b/gdb/testsuite/gdb.trace/tfile.c
> @@ -64,12 +64,12 @@ add_memory_block (char *addr, int size)
>  }
>  
>  void
> -write_basic_trace_file (void)
> +write_basic_trace_file (char *file_name)
>  {
>    int fd, int_x;
>    short short_x;
>  
> -  fd = start_trace_file ("basic.tf");
> +  fd = start_trace_file (file_name);
>  
>    /* The next part of the file consists of newline-separated lines
>       defining status, tracepoints, etc.  The section is terminated by
> @@ -147,14 +147,14 @@ bin2hex (const char *bin, char *hex, int count)
>  }
>  
>  void
> -write_error_trace_file (void)
> +write_error_trace_file (char *file_name)
>  {
>    int fd;
>    const char made_up[] = "made-up error";
>    int len = sizeof (made_up) - 1;
>    char *hex = alloca (len * 2 + 1);
>  
> -  fd = start_trace_file ("error.tf");
> +  fd = start_trace_file (file_name);
>  
>    /* The next part of the file consists of newline-separated lines
>       defining status, tracepoints, etc.  The section is terminated by
> @@ -205,9 +205,12 @@ done_making_trace_files (void)
>  int
>  main (int argc, char **argv, char **envp)
>  {
> -  write_basic_trace_file ();
> +  char *basic_file_name = argv[1];
> +  char *error_file_name = argv[2];

This should have an argc check, for when you run the program
manually from the command line, with no args, for example.

(I'd have split this change out to that new not yet submitted test you
mention -- what if review of that new test finds a different approach
would be better?  No big harm done in this case, so okay.)

>  
> -  write_error_trace_file ();
> +  write_basic_trace_file (basic_file_name);
> +
> +  write_error_trace_file (error_file_name);

>  
>    done_making_trace_files ();
>  
> diff --git a/gdb/testsuite/gdb.trace/tfile.exp b/gdb/testsuite/gdb.trace/tfile.exp
> index a2c5c1d..e2b5263 100644
> --- a/gdb/testsuite/gdb.trace/tfile.exp
> +++ b/gdb/testsuite/gdb.trace/tfile.exp
> @@ -22,12 +22,6 @@
>  
>  load_lib "trace-support.exp";
>  
> -if [target_info exists gdb,nofileio] {
> -    verbose "Skipping tfile.exp because of no fileio capabilities."
> -    continue
> -}
> -
> -
>  gdb_exit
>  gdb_start
>  standard_testfile
> @@ -41,18 +35,15 @@ gdb_reinitialize_dir $srcdir/$subdir
>  # Make sure we are starting fresh.
>  remote_file host delete basic.tf
>  remote_file host delete error.tf
> +remote_file target delete b.tf
> +remote_file target delete e.tf
>  
> -gdb_load $binfile
> -
> -runto_main
> +remote_exec target "$binfile b.tf e.tf"
> +# Copy tracefile from target to host.
> +remote_download host [remote_upload target b.tf ] basic.tf
> +remote_download host [remote_upload target e.tf ] error.tf

Spurious spaces before ']'.  But, what's going on here?
The target produces b.tf, and e.tf.  Then those are copied
to the host with remote_upload.  And then copied again
to the host, as basic.tf, and error.tf.  Why this juggling?

>  
> -gdb_test "break done_making_trace_files" ".*" ""
> -
> -gdb_test "continue" ".*" ""
> -
> -# tsave command would be tested here...
> -
> -gdb_test "continue" ".*" ""
> +gdb_load $binfile
>  
>  # Program has presumably exited, now target a trace file it created.

-- 
Pedro Alves