* [review] [gdb/testsuite] Make inferior_exited_re match a single line
2020-02-03 15:15 [review] [gdb/testsuite] Make inferior_exited_re match a single line Tom de Vries (Code Review)
@ 2020-02-04 4:38 ` Simon Marchi (Code Review)
2020-02-04 9:31 ` Tom de Vries (Code Review)
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2020-02-04 4:38 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/764
......................................................................
Patch Set 1:
(1 comment)
| --- gdb/testsuite/lib/gdb.exp
| +++ gdb/testsuite/lib/gdb.exp
| @@ -112,18 +112,18 @@ global env
| if ![info exists env(EXEEXT)] {
| set EXEEXT ""
| } else {
| set EXEEXT $env(EXEEXT)
| }
|
| set octal "\[0-7\]+"
|
| -set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
| +set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r]*\\) exited)"
PS1, Line 120:
Usually I would escape the closing square bracket, like what is done
in `\[0-9\]` just before on the same line. But I presume it's not
strictly necessary, and that the regexp engine sees that this closing
bracket doesn't match any opening bracket, and therefore treats it
like a normal character?
For consistency, I would suggest escaping it, but the patch LGTM
either way.
|
| # A regular expression that matches a value history number.
| # E.g., $1, $2, etc.
| set valnum_re "\\\$$decimal"
|
| ### Only procedures should come after this point.
|
| #
| # gdb_version -- extract and print the version number of GDB
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
Gerrit-Change-Number: 764
Gerrit-PatchSet: 1
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 04 Feb 2020 04:38:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [review] [gdb/testsuite] Make inferior_exited_re match a single line
2020-02-03 15:15 [review] [gdb/testsuite] Make inferior_exited_re match a single line Tom de Vries (Code Review)
2020-02-04 4:38 ` Simon Marchi (Code Review)
@ 2020-02-04 9:31 ` Tom de Vries (Code Review)
2020-02-04 9:32 ` [review v2] " Tom de Vries (Code Review)
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries (Code Review) @ 2020-02-04 9:31 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Tom de Vries has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/764
......................................................................
Patch Set 1:
(1 comment)
| --- gdb/testsuite/lib/gdb.exp
| +++ gdb/testsuite/lib/gdb.exp
| @@ -112,18 +112,18 @@ global env
| if ![info exists env(EXEEXT)] {
| set EXEEXT ""
| } else {
| set EXEEXT $env(EXEEXT)
| }
|
| set octal "\[0-7\]+"
|
| -set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
| +set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r]*\\) exited)"
PS1, Line 120:
It was a typo, thanks for noticing that. I've retested and will submit
the updated patch.
I've tried to understand why not escaping the closing square bracket
still works, and I think it happens before a string is interpreted as
regexp, in the substitutions.
The escape of the opening square bracket prevents command
substitution, but after substitution is done we're left with the same
string, whether we escaped the closing square bracket or not:
...
$ tclsh
% set line1 "\[bla]"
[bla]
% set line2 "\[bla\]"
[bla]
% string equal $line1 $line2
1
...
|
| # A regular expression that matches a value history number.
| # E.g., $1, $2, etc.
| set valnum_re "\\\$$decimal"
|
| ### Only procedures should come after this point.
|
| #
| # gdb_version -- extract and print the version number of GDB
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
Gerrit-Change-Number: 764
Gerrit-PatchSet: 1
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Comment-Date: Tue, 04 Feb 2020 09:31:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [review v2] [gdb/testsuite] Make inferior_exited_re match a single line
2020-02-03 15:15 [review] [gdb/testsuite] Make inferior_exited_re match a single line Tom de Vries (Code Review)
2020-02-04 4:38 ` Simon Marchi (Code Review)
2020-02-04 9:31 ` Tom de Vries (Code Review)
@ 2020-02-04 9:32 ` Tom de Vries (Code Review)
2020-02-04 14:23 ` Simon Marchi (Code Review)
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Tom de Vries (Code Review) @ 2020-02-04 9:32 UTC (permalink / raw)
To: gdb-patches; +Cc: Simon Marchi
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/764
......................................................................
[gdb/testsuite] Make inferior_exited_re match a single line
The current inferior_exited_re regexp contains a '.*':
...
set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
...
This means that while matching a single line:
...
$ tclsh
% set re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
(?:\[Inferior [0-9]+ \(.*\) exited)
% set line "\[Inferior 1 (process 33) exited\]\n"
[Inferior 1 (process 33) exited]
% regexp $re $line
1
...
it also matches more than one line:
...
$ tclsh
% set re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
(?:\[Inferior [0-9]+ \(.*\) exited)
% set line "\[Inferior 1 (process 33) exited\]\n\[Inferior 2 (process 44) exited\]\n"
[Inferior 1 (process 33) exited]
[Inferior 2 (process 44) exited]
% regexp $re $line
1
...
Fix this by using "\[^\n\r\]*" instead of ".*".
Build and reg-tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-02-01 Tom de Vries <tdevries@suse.de>
* lib/gdb.exp (inferior_exited_re): Use "\[^\n\r\]*" instead of ".*".
Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
---
M gdb/testsuite/lib/gdb.exp
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 25bed76..eb1d145 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -117,7 +117,7 @@
set octal "\[0-7\]+"
-set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
+set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r\]*\\) exited)"
# A regular expression that matches a value history number.
# E.g., $1, $2, etc.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
Gerrit-Change-Number: 764
Gerrit-PatchSet: 2
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-CC: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 7+ messages in thread* [review v2] [gdb/testsuite] Make inferior_exited_re match a single line
2020-02-03 15:15 [review] [gdb/testsuite] Make inferior_exited_re match a single line Tom de Vries (Code Review)
` (2 preceding siblings ...)
2020-02-04 9:32 ` [review v2] " Tom de Vries (Code Review)
@ 2020-02-04 14:23 ` Simon Marchi (Code Review)
2020-02-04 16:32 ` [pushed] " Sourceware to Gerrit sync (Code Review)
2020-02-04 16:32 ` Sourceware to Gerrit sync (Code Review)
5 siblings, 0 replies; 7+ messages in thread
From: Simon Marchi (Code Review) @ 2020-02-04 14:23 UTC (permalink / raw)
To: Tom de Vries, gdb-patches
Simon Marchi has posted comments on this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/764
......................................................................
Patch Set 2: Code-Review+2
(1 comment)
| --- gdb/testsuite/lib/gdb.exp
| +++ gdb/testsuite/lib/gdb.exp
| @@ -112,18 +112,18 @@ global env
| if ![info exists env(EXEEXT)] {
| set EXEEXT ""
| } else {
| set EXEEXT $env(EXEEXT)
| }
|
| set octal "\[0-7\]+"
|
| -set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
| +set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r]*\\) exited)"
PS1, Line 120:
Eh, you're right, what I said didn't make sense. We escape the
opening bracket so it's not interpreted as a procedure call, so it can
be interpreted as a character group. But still, it's the same: if
there's an unescaped closing bracket that doesn't match any unescaped
opening bracket, tcl will treat it as a regular character.
|
| # A regular expression that matches a value history number.
| # E.g., $1, $2, etc.
| set valnum_re "\\\$$decimal"
|
| ### Only procedures should come after this point.
|
| #
| # gdb_version -- extract and print the version number of GDB
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
Gerrit-Change-Number: 764
Gerrit-PatchSet: 2
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-Comment-Date: Tue, 04 Feb 2020 14:23:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Tom de Vries <tdevries@suse.de>
Comment-In-Reply-To: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-MessageType: comment
^ permalink raw reply [flat|nested] 7+ messages in thread* [pushed] [gdb/testsuite] Make inferior_exited_re match a single line
2020-02-03 15:15 [review] [gdb/testsuite] Make inferior_exited_re match a single line Tom de Vries (Code Review)
` (3 preceding siblings ...)
2020-02-04 14:23 ` Simon Marchi (Code Review)
@ 2020-02-04 16:32 ` Sourceware to Gerrit sync (Code Review)
2020-02-04 16:32 ` Sourceware to Gerrit sync (Code Review)
5 siblings, 0 replies; 7+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2020-02-04 16:32 UTC (permalink / raw)
To: Tom de Vries, gdb-patches; +Cc: Simon Marchi
Sourceware to Gerrit sync has submitted this change.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/764
......................................................................
[gdb/testsuite] Make inferior_exited_re match a single line
The current inferior_exited_re regexp contains a '.*':
...
set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
...
This means that while matching a single line:
...
$ tclsh
% set re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
(?:\[Inferior [0-9]+ \(.*\) exited)
% set line "\[Inferior 1 (process 33) exited\]\n"
[Inferior 1 (process 33) exited]
% regexp $re $line
1
...
it also matches more than one line:
...
$ tclsh
% set re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
(?:\[Inferior [0-9]+ \(.*\) exited)
% set line "\[Inferior 1 (process 33) exited\]\n\[Inferior 2 (process 44) exited\]\n"
[Inferior 1 (process 33) exited]
[Inferior 2 (process 44) exited]
% regexp $re $line
1
...
Fix this by using "\[^\n\r\]*" instead of ".*".
Build and reg-tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-02-04 Tom de Vries <tdevries@suse.de>
* lib/gdb.exp (inferior_exited_re): Use "\[^\n\r\]*" instead of ".*".
Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
---
M gdb/testsuite/ChangeLog
M gdb/testsuite/lib/gdb.exp
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bc61679..8fcf67b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
2020-02-04 Tom de Vries <tdevries@suse.de>
+ * lib/gdb.exp (inferior_exited_re): Use "\[^\n\r\]*" instead of ".*".
+
+2020-02-04 Tom de Vries <tdevries@suse.de>
+
* lib/gdb.exp (inferior_exited_re): Use non-capturing parentheses.
2020-02-03 Rogerio A. Cardoso <rcardoso@linux.ibm.com>
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 25bed76..eb1d145 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -117,7 +117,7 @@
set octal "\[0-7\]+"
-set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
+set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r\]*\\) exited)"
# A regular expression that matches a value history number.
# E.g., $1, $2, etc.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
Gerrit-Change-Number: 764
Gerrit-PatchSet: 3
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: merged
^ permalink raw reply [flat|nested] 7+ messages in thread* [pushed] [gdb/testsuite] Make inferior_exited_re match a single line
2020-02-03 15:15 [review] [gdb/testsuite] Make inferior_exited_re match a single line Tom de Vries (Code Review)
` (4 preceding siblings ...)
2020-02-04 16:32 ` [pushed] " Sourceware to Gerrit sync (Code Review)
@ 2020-02-04 16:32 ` Sourceware to Gerrit sync (Code Review)
5 siblings, 0 replies; 7+ messages in thread
From: Sourceware to Gerrit sync (Code Review) @ 2020-02-04 16:32 UTC (permalink / raw)
To: Tom de Vries, Simon Marchi, gdb-patches
The original change was created by Tom de Vries.
Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/764
......................................................................
[gdb/testsuite] Make inferior_exited_re match a single line
The current inferior_exited_re regexp contains a '.*':
...
set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
...
This means that while matching a single line:
...
$ tclsh
% set re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
(?:\[Inferior [0-9]+ \(.*\) exited)
% set line "\[Inferior 1 (process 33) exited\]\n"
[Inferior 1 (process 33) exited]
% regexp $re $line
1
...
it also matches more than one line:
...
$ tclsh
% set re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
(?:\[Inferior [0-9]+ \(.*\) exited)
% set line "\[Inferior 1 (process 33) exited\]\n\[Inferior 2 (process 44) exited\]\n"
[Inferior 1 (process 33) exited]
[Inferior 2 (process 44) exited]
% regexp $re $line
1
...
Fix this by using "\[^\n\r\]*" instead of ".*".
Build and reg-tested on x86_64-linux.
gdb/testsuite/ChangeLog:
2020-02-04 Tom de Vries <tdevries@suse.de>
* lib/gdb.exp (inferior_exited_re): Use "\[^\n\r\]*" instead of ".*".
Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
---
M gdb/testsuite/ChangeLog
M gdb/testsuite/lib/gdb.exp
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index bc61679..8fcf67b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,5 +1,9 @@
2020-02-04 Tom de Vries <tdevries@suse.de>
+ * lib/gdb.exp (inferior_exited_re): Use "\[^\n\r\]*" instead of ".*".
+
+2020-02-04 Tom de Vries <tdevries@suse.de>
+
* lib/gdb.exp (inferior_exited_re): Use non-capturing parentheses.
2020-02-03 Rogerio A. Cardoso <rcardoso@linux.ibm.com>
diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 25bed76..eb1d145 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -117,7 +117,7 @@
set octal "\[0-7\]+"
-set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(.*\\) exited)"
+set inferior_exited_re "(?:\\\[Inferior \[0-9\]+ \\(\[^\n\r\]*\\) exited)"
# A regular expression that matches a value history number.
# E.g., $1, $2, etc.
--
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Id7b1dcecd8c7fda3d1ab34b4fa1364d301748333
Gerrit-Change-Number: 764
Gerrit-PatchSet: 3
Gerrit-Owner: Tom de Vries <tdevries@suse.de>
Gerrit-Reviewer: Simon Marchi <simon.marchi@polymtl.ca>
Gerrit-Reviewer: Tom de Vries <tdevries@suse.de>
Gerrit-MessageType: newpatchset
^ permalink raw reply [flat|nested] 7+ messages in thread