public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Anton Kolesov <Anton.Kolesov@synopsys.com>
To: gdb-patches@sourceware.org
Cc: Jeremy Bennett <Jeremy.bennett@embecosm.com>,
	Pedro Alves <palves@redhat.com>
Subject: [PATCH v3] testsuite: Treat an empty string in needs_status_wrapper as false
Date: Tue, 22 Oct 2013 10:51:00 -0000	[thread overview]
Message-ID: <1382439080-29410-1-git-send-email-Anton.Kolesov@synopsys.com> (raw)
In-Reply-To: <52650EA6.6030903@redhat.com>

Updated according to feedback.

GDB test suite considers [target_info needs_status_wrapper] to be false if
it is unset or have a zero value. The former is achieved by using [target_info
exists needs_status_wrapper]. GCC test suite on the other hand do not use
"exists" but compares to an empty string. This doesn't make difference if
value is unset, as unset value is treated as an empty string, but makes a
difference if value was set to and empty string. In that case if
needs_status_wrapper was set to an empty string, then GCC test suite will
not use status wrapper, but GDB test suite will use it. Dejagnu's own
remote.exp uses a comparison with an empty string. Though for some reason
Dejagnu unlike GCC and GDB test suite doesn't treat a zero as a false.

This patch makes GDB test suite treat an empty string in
needs_status_wrapper the same way as it is done by GCC test suite and
Dejagnu. It also extracts those checks into separate function
'gdb_needs_status_wrapper'.

gdb/testsuite/ChangeLog:

2013-10-22  Anton Kolesov <Anton.Kolesov@synopsys.com>

	* lib/gdb.exp (gdb_needs_status_wrapper): New function.
	* lib/gdb.exp (gdb_compile, gdb_wrapper_init): Replace inline checks for
	needs_status_wrapper value with call to a new function.
	* lib/java.exp (java_init): Ditto.
---
 gdb/testsuite/lib/gdb.exp  | 25 +++++++++++++++++++++----
 gdb/testsuite/lib/java.exp |  2 +-
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 3efd539..7001c8d 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2510,8 +2510,7 @@ proc gdb_wrapper_init { args } {
 
     if { $gdb_wrapper_initialized == 1 } { return; }
 
-    if {[target_info exists needs_status_wrapper] && \
-	    [target_info needs_status_wrapper] != "0"} {
+    if {[gdb_needs_status_wrapper] == 1} {
 	set result [build_wrapper "testglue.o"]
 	if { $result != "" } {
 	    set gdb_wrapper_file [lindex $result 0]
@@ -2609,8 +2608,7 @@ proc gdb_compile {source dest type options} {
 
     if { $gdb_wrapper_initialized == 0 } { gdb_wrapper_init }
 
-    if {[target_info exists needs_status_wrapper] && \
-	    [target_info needs_status_wrapper] != "0" && \
+    if {[gdb_needs_status_wrapper] == 1 && \
 	    [info exists gdb_wrapper_file]} {
 	lappend options "libs=${gdb_wrapper_file}"
 	lappend options "ldflags=${gdb_wrapper_flags}"
@@ -4492,5 +4490,24 @@ proc using_fission { } {
     return [regexp -- "-gsplit-dwarf" $debug_flags]
 }
 
+# There seems to be a lack of clear definition of what value of target_info
+# needs_status_wrapper should be considered as False and what as True. Dejagnu
+# in remote.exp makes a check comparing with an empty string. Which means that
+# False is an empty string or unset value. GCC test suite does the same, but it
+# also treats zero as a False. GDB previously was using zero and the unset
+# value as False, but unlike others it was treating an explicitly set empty
+# string as True. This function applies the same logic as GCC test suite for a
+# compatibility reasons and should be used instead of any direct checks for a
+# needs_status_wrapper value.
+# Return 1 if status wrapper is required, return 0 otherwise.
+proc gdb_needs_status_wrapper { } {
+	if {[target_info needs_status_wrapper] != "" && \
+	    [target_info needs_status_wrapper] != "0" } {
+		return 1
+	} else {
+		return 0
+	}
+}
+
 # Always load compatibility stuff.
 load_lib future.exp
diff --git a/gdb/testsuite/lib/java.exp b/gdb/testsuite/lib/java.exp
index 19b1eee..8e6ff23 100644
--- a/gdb/testsuite/lib/java.exp
+++ b/gdb/testsuite/lib/java.exp
@@ -70,7 +70,7 @@ proc java_init { args } {
 
     set wrapper_file ""
     set wrap_compile_flags ""
-    if [target_info exists needs_status_wrapper] {
+    if {[gdb_needs_status_wrapper] == 1} {
 	set result [build_wrapper "testglue.o"]
 	if { $result != "" } {
 	    set wrapper_file [lindex $result 0]
-- 
1.8.4.1

  reply	other threads:[~2013-10-22 10:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-17 12:59 [PATCH] testsuite: Treat empty string in needs_status_wrapper as a false value Anton Kolesov
2013-10-17 15:07 ` Anton Kolesov
2013-10-21 11:17   ` [PATCH v2] testsuite: Treat an empty string in needs_status_wrapper as false Anton Kolesov
2013-10-21 11:23     ` Pedro Alves
2013-10-22 10:51       ` Anton Kolesov [this message]
2013-10-24 22:07         ` [PATCH v3] " Pedro Alves
2013-10-25  9:25           ` Anton Kolesov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1382439080-29410-1-git-send-email-Anton.Kolesov@synopsys.com \
    --to=anton.kolesov@synopsys.com \
    --cc=Jeremy.bennett@embecosm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).