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

[Jim Meyering]

Bernhard Voelker wrote:
> On 04/04/2012 05:29 PM, Jim Meyering wrote:
>> Bernhard Voelker wrote:
>>> I think this code hasn't been run for a very long time,
>>> because RUN_LONG_TESTS is not set anywhere.
>>>
>>>   if test -n "$RUN_LONG_TESTS"; then
>>>     # Take them in pairs.
>>>
>>> So shouldn't this ~20sec part of the test be moved into
>>> a very_expensive_ guard?
>>
>> Good idea.
>> Wow.  That code hasn't been touch since the last millennium ;-)
>>
>>> And if that part runs, then the test fails (at least here)
>>> because of parenb and cread options.
>>>
>>> Do they have to be excluded?
>>
>> That makes sense, since parenb and cread are already exempted in
>> the preceding one-at-a-time on/off tests.
>
> Besides factoring out the expensive code, I found out
> that "iutf8" was missing in the option list.
>
> * [PATCH 1/2] tests: add iutf8 option to misc/stty
> * [PATCH 2/2] tests: factor out expensive "pairs" code of misc/stty
>
> I'm not happy that the REV_* list is doubled now,
> but I'm optimistic that with the "See also:" comment
> we're quite safe that both places will be updated next
> time.
...
> Subject: [PATCH 1/2] tests: add iutf8 option to misc/stty
>
> * tests/misc/stty: Add iutf8 to the list of REV_* options.
> That option has been implemented in commit v5.2.1-193-g733e79e.
> ---
>  tests/misc/stty |   14 +++++++-------
>  1 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tests/misc/stty b/tests/misc/stty
> index 1b082c2..cfc70c1 100755
> --- a/tests/misc/stty
> +++ b/tests/misc/stty
> @@ -29,13 +29,13 @@ 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_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
> +REV_iutf8=1 REV_opost=1 REV_olcuc=1 REV_ocrnl=1 REV_onlcr=1 REV_onocr=1
...

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

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

  r=$(perl -lne '/^ *{"(.*?)",.*\bREV\b/ and print "REV_$1=1"' \
        $abs_top_srcdir/stty.c)
  # Ensure that there are at least 62, i.e., so that reformatting
  # the source does not make this test a no-op.
  test 62 -lt $(echo "$r"|wc -l) || framework_failure_
  eval "$r"

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"
}

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/')