Re: [PATCH] Share gdbservre setting for board files native-*gdbserver.exp

[Pedro Alves <palves at redhat dot com>]

On 07/05/2013 10:08 AM, Yao Qi wrote:
> Hi,
> native-gdbserver.exp, native-extended-gdbserver.exp and native-stdio.exp
> duplicate some bits, and this patch is to move this duplication to a new
> file board/gdbserver.exp.  Other board files can use it via
> 'load_board_description "gdbserver"'.
> 
> Note that I notice that ${board}_upload is not defined in
> native-extended-gdbserver.exp, I am not sure it is intentional or just an
> oversight.

I don't have any recollection of leaving it out on purpose, while
leaving in ${board}_download.

I believe we could say that either having ${board}_download was
an oversight, or not having ${board}_upload was an oversight.
It's indeed odd to have only one of those.

But I believe this bit (native-extended-gdbserver.exp):

 # By default, dejagnu makes the board remote unless the board name
 # matches localhost.  Force it to be NOT remote.
 global board
 global board_info
 set board_info($board,isremote) 0

makes it so that the !remote branch in dejagnu is taken in
both remote_download and remote_upload:

/usr/share/dejagnu/remote.exp:

 proc remote_download { dest file args } {
 (...)
    if { ![is_remote $dest] } {
	} else {
	    set result [catch "exec cp -p $file $destfile" output]


 proc remote_upload {dest srcfile args} {
 (...)
    if { ![is_remote $dest] } {
 (...)
	set result [catch "exec cp -p $srcfile $destfile" output]
	return $destfile
    }
 }

Defining both the empty-ish ${board}_download and ${board}_upload
in the shared board should do no harm.

> 
> With the patch applied, I tried board files native-stdio-gdbserver,
> native-gdbserver, native-extended-gdbserver and
> native-stdio-gdbserver.  They still behave correctly.
> 
> gdb/testsuite:
> 

This is OK.  It's OK too if you'd like to share remote_upload (and it
works, of course).

> 2013-07-05  Yao Qi  <yao@codesourcery.com>
...
> 	* boards/gdbserver.exp: New file.

How about we call this gdbserver-base.exp?  Otherwise I can already picture
people seeing that file and trying out "--target_board=gdbserver" ...

> +# This file is shared for other  dejagnu "board files" which are used
> +# to run the testsuite with gdbserver.

s/shared for/shared with/ ?  or:
s/This file is shared for other/This file has common bits shared between other/

Spurious double space before dejagnu.  s/which/that/ (restrictive clause).

Suggest:

# This file has common bits shared between other dejagnu "board files"
# that are used to run the testsuite with gdbserver.

-- 
Pedro Alves