Re: [PATCH] refactor expensive code in misc/stty [was: amendments to backtick-removing series]

[Bernhard Voelker]

CC'ing Sami as he sent another note privately.

On 04/05/2012 10:01 AM, Jim Meyering wrote:

> So iutf8 was added to stty.c (by me, 8 years ago), but not here,
> so never exposed to these tests.  Good catch.

Thanks.

> What do you think about eval'ing the output of the command
> just above, instead.  Then this won't happen again.

I thought about that (but read on) ...


> Or better, a function in init.cfg,
> 
> stty_reversible_settings()
> {
>   local r
>   r=$(perl -lne '/^ *{"(.*?)",.*\bREV\b/ and print $1' \
>       $abs_top_srcdir/src/stty.c)
>   # Ensure that there are at least 62, i.e., so we're alerted if
>   # reformatting the source empties the list.
>   test 62 -lt $(echo "$r"|wc -l) || framework_failure_
>   echo "$r"
> }

a) s/lt/le/ ;-)

> 
> Then, you could add this one-liner in each of those two files,
> in place of the hard-coded list of REV_... variable settings:
> 
>   # Set REV_$S=1 for each reversible stty setting, $S.
>   eval $(stty_reversible_settings |sed 's/^/REV_/;s/$/=1/')
> 

b) framework_failure_ in a subshell doesn't work.
c) I don't like eval too much, it's harder to read.

What about the following?

P.S. I could've even overloaded the function in init.cfg
to get the list when no arg is given, and otherwise test if
the arg is in the internal list, but I think this is safer.

Have a nice day,
Berny


>From b8e5e7e9f25d21284daffeca65634840dfb1d0c1 Mon Sep 17 00:00:00 2001
From: Bernhard Voelker <address@hidden>
Date: Thu, 5 Apr 2012 12:05:46 +0200
Subject: [PATCH] tests: factor out expensive "pairs" code of misc/stty

* tests/Makefile.am: Add misc/stty-pairs.
* tests/init.cfg (stty_reversible_settings): Add new function to get
the list of reversible options from stty.c.
* tests/init.cfg (stty_reversible_matches): Add new function to test
wether a given option is in the list of reversible options.
* tests/misc/stty: Factor out expensive "pairs" code into new test.
Use new stty_reversible_settings and stty_reversible_matches instead
of evaluating static REV_* variables.
* tests/misc/stty-pairs: Added new test. Code added from misc/stty.
Mark this as an expensive test. Skip 'parenb' and 'cread' options,
as these tests are known to fail. Like in misc/stty, also use
new stty_reversible_settings and stty_reversible_matches.
---
 tests/Makefile.am     |    1 +
 tests/init.cfg        |   27 +++++++++++++++++++++
 tests/misc/stty       |   42 ++------------------------------
 tests/misc/stty-pairs |   63 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 94 insertions(+), 39 deletions(-)
 create mode 100755 tests/misc/stty-pairs

diff --git a/tests/Makefile.am b/tests/Makefile.am
index 011051a..86afbc2 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -274,6 +274,7 @@ TESTS =                                             \
   misc/stdbuf                                  \
   misc/stty                                    \
   misc/stty-invalid                            \
+  misc/stty-pairs                              \
   misc/stty-row-col                            \
   misc/su-fail                                 \
   misc/sum                                     \
diff --git a/tests/init.cfg b/tests/init.cfg
index 7bcd512..08645d1 100644
--- a/tests/init.cfg
+++ b/tests/init.cfg
@@ -241,6 +241,33 @@ rwx_to_mode_()
   echo "=$u$g$o"
 }

+# Return the reversible settings from stty.c.
+# Fill the variable REV which is needed by stty_reversible_matches.
+stty_reversible_settings()
+{
+  # Pad start with one blank for the first option
+  # to match in stty_reversible_matches.
+  REV=" "$(perl -lne '/^ *{"(.*?)",.*\bREV\b/ and print $1' \
+      $abs_top_srcdir/src/stty.c | tr '\n' ' ')
+  # Ensure that there are at least 62, i.e., so we're alerted if
+  # reformatting the source empties the list.
+  test 62 -le $(echo "$REV"|wc -w)  \
+    || framework_failure_ "too few reversible settings"
+}
+
+# Test if $1 is among the reversible stty options ($REV).
+stty_reversible_matches()
+{
+  case "$REV" in
+    "")
+      framework_failure_ "stty_reversible_settings() not called?";;
+    *" $1 "*)
+      return 0;;
+    *)
+      return 1;;
+  esac
+}
+
 skip_if_()
 {
   case $1 in
diff --git a/tests/misc/stty b/tests/misc/stty
index cfc70c1..578105d 100755
--- a/tests/misc/stty
+++ b/tests/misc/stty
@@ -23,20 +23,8 @@ print_ver_ stty
 require_controlling_input_terminal_
 trap '' TTOU # Ignore SIGTTOU

-# The following list of reversible options was generated with
-# grep -w REV stty.c|sed -n '/^  {"/{s//REV_/;s/".*/=1/;p;}'|fmt
-REV_parenb=1 REV_parodd=1 REV_hupcl=1 REV_hup=1 REV_cstopb=1 REV_cread=1
-REV_clocal=1 REV_crtscts=1 REV_ignbrk=1 REV_brkint=1 REV_ignpar=1
-REV_parmrk=1 REV_inpck=1 REV_istrip=1 REV_inlcr=1 REV_igncr=1 REV_icrnl=1
-REV_ixon=1 REV_ixoff=1 REV_tandem=1 REV_iuclc=1 REV_ixany=1 REV_imaxbel=1
-REV_iutf8=1 REV_opost=1 REV_olcuc=1 REV_ocrnl=1 REV_onlcr=1 REV_onocr=1
-REV_onlret=1 REV_ofill=1 REV_ofdel=1 REV_isig=1 REV_icanon=1 REV_iexten=1
-REV_echo=1 REV_echoe=1 REV_crterase=1 REV_echok=1 REV_echonl=1
-REV_noflsh=1 REV_xcase=1 REV_tostop=1 REV_echoprt=1 REV_prterase=1
-REV_echoctl=1 REV_ctlecho=1 REV_echoke=1 REV_crtkill=1 REV_evenp=1
-REV_parity=1 REV_oddp=1 REV_nl=1 REV_cooked=1 REV_raw=1 REV_pass8=1
-REV_litout=1 REV_cbreak=1 REV_decctlq=1 REV_tabs=1 REV_lcase=1 REV_LCASE=1
-
+# Get the reversible settings from stty.c.
+stty_reversible_settings

 saved_state=.saved-state
 stty --save > $saved_state || fail=1
@@ -70,35 +58,11 @@ for opt in $options; do

   # Likewise, 'stty -cread' would fail, so skip that, too.
   test $opt = cread && continue
-  eval rev=\$REV_$opt
-  if test -n "$rev"; then
+  if $(stty_reversible_matches "$opt") ; then
     stty -$opt || { fail=1; echo -$opt; }
   fi
 done

-if test -n "$RUN_LONG_TESTS"; then
-  # Take them in pairs.
-  for opt1 in $options; do
-    echo .|tr -d '\n'
-    for opt2 in $options; do
-
-      stty $opt1 $opt2 || fail=1
-
-      eval rev1=\$REV_$opt1
-      eval rev2=\$REV_$opt2
-      if test -n "$rev1"; then
-        stty -$opt1 $opt2 || fail=1
-      fi
-      if test -n "$rev2"; then
-        stty $opt1 -$opt2 || fail=1
-      fi
-      if test "$rev1$rev2" = 11; then
-        stty -$opt1 -$opt2 || fail=1
-      fi
-    done
-  done
-fi
-
 stty $(cat $saved_state)

 Exit $fail
diff --git a/tests/misc/stty-pairs b/tests/misc/stty-pairs
new file mode 100755
index 0000000..76808d4
--- /dev/null
+++ b/tests/misc/stty-pairs
@@ -0,0 +1,63 @@
+#!/bin/sh
+# Make sure stty can parse most of its options - in pairs [expensive].
+
+# Copyright (C) 1998-2012 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/init.sh"; path_prepend_ ../src
+print_ver_ stty
+
+expensive_
+
+# Make sure there's a tty on stdin.
+require_controlling_input_terminal_
+trap '' TTOU # Ignore SIGTTOU
+
+# Get the reversible settings from stty.c.
+stty_reversible_settings
+
+saved_state=.saved-state
+stty --save > $saved_state || fail=1
+stty $(cat $saved_state) || fail=1
+
+# Build a list of all boolean options stty accepts on this system.
+# Don't depend on terminal width.  Put each option on its own line,
+# remove all non-boolean ones, remove 'parenb' and 'cread' explicitly,
+# then remove any leading hyphens.
+sed_del='/^speed/d;/^rows/d;/^columns/d;/ = /d;s/parenb//;s/cread//'
+options=$(stty -a | tr -s ';' '\n' | sed "s/^ //;$sed_del;s/-//g")
+
+# Take them in pairs, with and without the leading '-'.
+for opt1 in $options; do
+  for opt2 in $options; do
+
+    stty $opt1 $opt2 || fail=1
+
+    if $(stty_reversible_matches "$opt1") ; then
+      stty -$opt1 $opt2 || fail=1
+    fi
+    if $(stty_reversible_matches "$opt2"); then
+      stty $opt1 -$opt2 || fail=1
+    fi
+    if $(stty_reversible_matches "$opt1") \
+    && $(stty_reversible_matches "$opt2"); then
+      stty -$opt1 -$opt2 || fail=1
+    fi
+  done
+done
+
+stty $(cat $saved_state)
+
+Exit $fail
-- 
1.7.7