Re: PATCH: simplify regular expressions

[Jacob Bachmeyer]

Ben Elliston wrote:
> This patch is further to:
> http://lists.gnu.org/archive/html/dejagnu/2018-12/msg00042.html
>
> I would be grateful for a careful review from someone.
>
> Thanks,
> Ben
>
> 2018-12-10  Ben Elliston  <address@hidden>
>
>         * config/gdb-comm.exp, config/gdb_stub.exp, config/vxworks.exp,
>         lib/dg.exp, lib/framework.exp, lib/ftp.exp, lib/kermit.exp,
>         lib/rlogin.exp, lib/target.exp, lib/telnet.exp, runtest.exp,
>         testsuite/lib/libsup.exp: Simplify some regular expressions in
>         constant strings by placing them inside braces instead of
>         quotes. This allows one level of backslash quoting to be removed.
>
> diff --git a/config/gdb-comm.exp b/config/gdb-comm.exp
> [...]
> diff --git a/config/gdb_stub.exp b/config/gdb_stub.exp
>   
[Andreas Schwab mentioned some errors and missed simplifications that I 
will not repeat.]

GDB has changed somewhat over the years; are the strings those modules 
seek still valid or do we need to accept both "old" and "new" message 
sets?  Should DejaGnu eventually support GDB's MI mode, possibly in 
another module?

> diff --git a/lib/ftp.exp b/lib/ftp.exp
> index e1cc1a9..ad3d8e9 100644
> --- a/lib/ftp.exp
> +++ b/lib/ftp.exp
> @@ -186,7 +186,7 @@ proc ftp_download {host localfile remotefile} {
>                 set loop 0
>                 set remotefile ""
>             }
> -           -re "(^|\[\r\n\])150.*connection for (.*) 
> \[(\]\[0-9.,\]+\\)\[\r\n\]" {
> +           -re {(^|[\r\n])150.*connection for (.*) [(][0-9.,]+\)[\r\n]} {
>                 set remotefile $expect_out(2,string)
>                 exp_continue
>             }
>   

Why use [(] for the open paren and \) for the close paren?  I suggest 
changing that piece to [(][0-9.,]+[)] or \([0-9.,]+\) for readability.

> diff --git a/runtest.exp b/runtest.exp
> index 0bfca7d..eb18d7d 100644
> --- a/runtest.exp
> +++ b/runtest.exp
> @@ -1563,16 +1563,16 @@ proc process_target_variants { target_list } {
>      set result {}
>      foreach x $target_list {
>         if {[regexp "\\(" $x]} {
> -           regsub "^.*\\((\[^()\]*)\\)$" "$x" "\\1" variant_list
> -           regsub "\\(\[^(\]*$" "$x" "" x
> +           regsub {^.*\(([^()]*)\)$} $x {\1} variant_list
> +           regsub "\\(\[^(\]*$" $x "" x
>             set list [process_target_variants $x]
>             set result {}
>             foreach x $list {
>                 set result [concat $result [iterate_target_variants $x [split 
> $variant_list ","]]]
>             }
>         } elseif {[regexp "\{" $x]} {
> -           regsub "^.*\{(\[^\{\}\]*)\}$" "$x" "\\1" variant_list
> -           regsub "\{\[^\{\]*$" "$x" "" x
> +           regsub "^.*\{(\[^\{\}\]*)\}$" $x {\1} variant_list
> +           regsub "\{\[^\{\]*$" $x "" x
>             set list [process_target_variants $x]
>             foreach x $list {
>                 foreach i [split $variant_list ","] {
>   

The regsub patterns here are all constant.  One is changed, but not the 
other three.


-- Jacob