Bernhard Voelker wrote: > On 04/05/2012 10:45 PM, Jim Meyering wrote: >> Bernhard Voelker wrote: >>> What about the following? >> >> It's already an expensive test, but even so, adding four $(...) subshell >> invocations in an inner loop that is run ~2-3000 times seems like it >> would have non-negligible cost, especially on systems where subshells >> are relatively expensive. >> >> I guess I prefer the original eval-using code, >> if you can find a way to make it work. > > ah yes, the subshells are silly. I just removed them. > The time for this test is now down from 13s with the > original master version to 8 seconds. > > I kept sticking to replace the eval-using code because - as > the variable store is now used from 2 test files - I liked > the idea that the "how the REVs are stored" is encapsulated > centrally in the 2 new shell functions in init.cfg. > >> BTW, your patch below was corrupted because something >> along the way converted non-leading TABs to spaces. > > Argh, it wasn't Thunderbird this time. I copied the patch from > an xterm, and during this, the TABs in Makefile.am have been > replaced (see also [1]). xclip is doing better.
>>From f71ed9ae1a9699e000a98c26dae4db96a628d657 Mon Sep 17 00:00:00 2001 > From: Bernhard Voelker <[email protected]> > Date: Thu, 5 Apr 2012 08:01:43 +0200 > Subject: [PATCH 1/2] tests: add iutf8 option to misc/stty ... Oops. This slipped off my radar. Sorry about that. The comment below wasn't quite right and I'd rather not use REV as a global variable. Too short. Similarly, I've renamed the function names slightly and added a trailing underscore to each. The trailing underscore is probably not absolutely necessary (and we haven't been very consistent about that), but it's a good practice. Here are proposed incremental changes: diff --git a/tests/init.cfg b/tests/init.cfg index 08645d1..2e43c16 100644 --- a/tests/init.cfg +++ b/tests/init.cfg @@ -241,26 +241,26 @@ 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' ' ') +# Set the global variable stty_reversible_ to a space-separated list of the +# reversible settings from stty.c. stty_reversible_ also starts and ends +# with a space. +stty_reversible_init_() +{ + # Pad start with one space for the first option to match in query function. + stty_reversible_=' '$(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) \ + test 62 -le $(echo "$stty_reversible_"|wc -w) \ || framework_failure_ "too few reversible settings" } -# Test if $1 is among the reversible stty options ($REV). -stty_reversible_matches() +# Test whether $1 is one of stty's reversible options. +stty_reversible_query_() { - case "$REV" in - "") - framework_failure_ "stty_reversible_settings() not called?";; + case $stty_reversible_ in + '') + framework_failure_ "stty_reversible_init_() not called?";; *" $1 "*) return 0;; *) diff --git a/tests/misc/stty b/tests/misc/stty index 10aaa0a..ae65656 100755 --- a/tests/misc/stty +++ b/tests/misc/stty @@ -24,7 +24,7 @@ require_controlling_input_terminal_ trap '' TTOU # Ignore SIGTTOU # Get the reversible settings from stty.c. -stty_reversible_settings +stty_reversible_init_ saved_state=.saved-state stty --save > $saved_state || fail=1 @@ -58,7 +58,7 @@ for opt in $options; do # Likewise, 'stty -cread' would fail, so skip that, too. test $opt = cread && continue - if stty_reversible_matches "$opt" ; then + if stty_reversible_query_ "$opt" ; then stty -$opt || { fail=1; echo -$opt; } fi done diff --git a/tests/misc/stty-pairs b/tests/misc/stty-pairs index 81ee46b..e59da04 100755 --- a/tests/misc/stty-pairs +++ b/tests/misc/stty-pairs @@ -26,7 +26,7 @@ require_controlling_input_terminal_ trap '' TTOU # Ignore SIGTTOU # Get the reversible settings from stty.c. -stty_reversible_settings +stty_reversible_init_ saved_state=.saved-state stty --save > $saved_state || fail=1 @@ -45,14 +45,14 @@ for opt1 in $options; do stty $opt1 $opt2 || fail=1 - if stty_reversible_matches "$opt1" ; then + if stty_reversible_query_ "$opt1" ; then stty -$opt1 $opt2 || fail=1 fi - if stty_reversible_matches "$opt2" ; then + if stty_reversible_query_ "$opt2" ; then stty $opt1 -$opt2 || fail=1 fi - if stty_reversible_matches "$opt1" \ - && stty_reversible_matches "$opt2" ; then + if stty_reversible_query_ "$opt1" \ + && stty_reversible_query_ "$opt2" ; then stty -$opt1 -$opt2 || fail=1 fi done
