[v5] Introduce target_is_gdbserver (was: v4)

On 2014-12-04 11:51 AM, Pedro Alves wrote:
> On 09/23/2014 04:19 PM, Simon Marchi wrote:
>> This patch introduces a function in gdbserver-support.exp to find out
>> whether the current target is GDBserver.
>>
>> The code was inspired from gdb.trace/qtro.exp, so it replaces the code
>> there by a call to the new function.
>>
>> New in v4:
>> - Return -1 on error, and check for -1 in qtro.exp.
>> - Use gdb_caching_proc to cache result.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.trace/qtro.exp: Replace gdbserver detection code by...
>> 	* lib/gdbserver-support.exp (target_is_gdbserver): New
>> 	fonction.
> 
> "function".  Or, "procedure".

Done.

>> diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
>> index 026a937..e3f421e 100644
>> --- a/gdb/testsuite/lib/gdbserver-support.exp
>> +++ b/gdb/testsuite/lib/gdbserver-support.exp
> 
> I don't think gdbserver-support.exp is always loaded on all
> targets?  By putting the procedure here, ISTM that non-GDBserver
> testing will ERROR when the procedure is called?

Damn, you are right (again). I moved it to gdb.exp in the updated patch below.

>> @@ -436,3 +436,28 @@ proc mi_gdbserver_start_multi { } {
>>  
>>      return [mi_gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
>>  }
>> +
>> +# Return 1 if the current remote target is an instance of gdbserver, 0
>> +# otherwise. Return -1 if there was an error and we can't tell.
> 
> s/gdbserver/our GDBserver/
> 
> Double space after period.

Done.

>> +
>> +gdb_caching_proc target_is_gdbserver {
>> +    global gdb_prompt
>> +
>> +    set is_gdbserver -1
>> +    set test "Probing for GDBserver"
>> +
>> +    gdb_test_multiple "monitor help" $test {
>> +	-re "The following monitor commands are supported.*Quit GDBserver.*$gdb_prompt $" {
>> +		set is_gdbserver 1
> 
> Indentation doesn't look right here.  Should be one tab + 4 spaces?

Right, done.

>> +	}
>> +	-re "$gdb_prompt $" {
>> +		set is_gdbserver 0
>> +	}
>> +    }
>> +
>> +    if { $is_gdbserver == -1 } {
>> +	verbose -log "Unable to tell whether we are using gdbserver or not."
>> +    }
>> +
>> +    return $is_gdbserver
>> +}
> 
> 
> Thanks,
> Pedro Alves
> 

Here is the updated patch.

From 918e76a8e4dfff49c89a8d69d6cc332e990dd4dc Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 23 Sep 2014 11:19:37 -0400
Subject: [PATCH v5] Introduce target_is_gdbserver

This patch introduces a function in gdbserver-support.exp to find out
whether the current target is GDBserver.

The code was inspired from gdb.trace/qtro.exp, so it replaces the code
there by a call to the new function.

New in v5:

	* Move target_is_gdbserver to gdb.exp.
	* Fix formatting and typos.

gdb/testsuite/ChangeLog:

	* gdb.trace/qtro.exp: Replace gdbserver detection code by...
	* lib/gdb.exp (target_is_gdbserver): New
	procedure.
---
 gdb/testsuite/gdb.trace/qtro.exp | 13 +------------
 gdb/testsuite/lib/gdb.exp        | 25 +++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 12 deletions(-)

On 12/05/2014 06:34 PM, Simon Marchi wrote:

> diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
> index a29b661..b420e00 100644
> --- a/gdb/testsuite/lib/gdb.exp
> +++ b/gdb/testsuite/lib/gdb.exp
> @@ -5004,5 +5004,30 @@ proc capture_command_output { command prefix } {
>      return $output_string
>  }
> 
> +# Return 1 if the current remote target is an instance of our GDBserver, 0
> +# otherwise.  Return -1 if there was an error and we can't tell.
> +
> +gdb_caching_proc target_is_gdbserver {
> +    global gdb_prompt
> +
> +    set is_gdbserver -1
> +    set test "Probing for GDBserver"
> +
> +    gdb_test_multiple "monitor help" $test {
> +	-re "The following monitor commands are supported.*Quit GDBserver.*$gdb_prompt $" {
> +	    set is_gdbserver 1
> +	}
> +	-re "$gdb_prompt $" {
> +	    set is_gdbserver 0
> +	}
> +    }
> +
> +    if { $is_gdbserver == -1 } {
> +	verbose -log "Unable to tell whether we are using GDBserver or not."
> +    }
> +
> +    return $is_gdbserver
> +}
> +

I think it'd be nice to have this close to gdb_is_target_remote.
How about moving it just below that one?

Anyway, this is OK.  Please push.

Thanks,
Pedro Alves