Re: [PATCH v5] Introduce target_is_gdbserver (was: v4)

[Simon Marchi <simon dot marchi at ericsson dot com>]

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(-)

diff --git a/gdb/testsuite/gdb.trace/qtro.exp b/gdb/testsuite/gdb.trace/qtro.exp
index 22b5051..d8ffc40 100644
--- a/gdb/testsuite/gdb.trace/qtro.exp
+++ b/gdb/testsuite/gdb.trace/qtro.exp
@@ -98,18 +98,7 @@ if { $traceframe_info_supported == -1 } {
 }

 # Check whether we're testing with our own GDBserver.
-set is_gdbserver -1
-set test "probe for GDBserver"
-gdb_test_multiple "monitor help" $test {
-    -re "The following monitor commands are supported.*debug-hw-points.*remote-debug.*GDBserver.*$gdb_prompt $" {
-	set is_gdbserver 1
-	pass $test
-    }
-    -re "$gdb_prompt $" {
-	set is_gdbserver 0
-	pass $test
-    }
-}
+set is_gdbserver [target_is_gdbserver]
 if { $is_gdbserver == -1 } {
     return -1
 }
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
+}
+
 # Always load compatibility stuff.
 load_lib future.exp
-- 
2.1.3