[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] [TCK] Convert scripts to also run with dash



On 11/15/2010 08:33 AM, Stefan Berger wrote:
> I am converting the shell-scripted tap tests to also run using the dash
> shell, fixing some other problems on the way as well.
> 
> Two of the tests' data files contained '\\1' in an sed command. This
> created char(1) with the dash, '\1' with the bash. I found it easier to
> replace the sed command with a different one than figuring out how to
> solve this escape problem so it works for both shells.
> 
>  
>  
> -function tap_fail() {
> -  echo "not ok $1 - ${2:0:66}"
> -  TAP_FAIL_LIST+="$1 "
> -  ((TAP_FAIL_CTR++))
> -  ((TAP_TOT_CTR++))
> +tap_fail() {
> +  txt=$(echo "$2" | gawk '{print substr($0,1,66)}')
> +  echo "not ok $1 - ${txt}"
> +  TAP_FAIL_LIST="$TAP_FAIL_LIST $1 "
> +  TAP_FAIL_CTR=$((TAP_FAIL_CTR+1))

Older versions of dash have a bug where you have to pre-expand a
variable to get this to work:

TAP_FAIL_CTR=$(($TAP_FAIL_CTR + 1))

(throughout the patch)

> @@ -112,8 +111,8 @@ function checkExpectedOutput() {
>  
>    exec 4<"${datafile}"
>  
> -  read <&4
> -  line="${REPLY}"
> +  IFS=""

Where do you restore IFS back to its original space-tab-newline setting?

>      while :; do
> -      read <&4
> -      line="${REPLY}"
> +      read line <&4
> +
> +      letter=$(echo ${line} | gawk '{print substr($1,1,1)}')
>  
> -      if [ "${line:0:1}" == "#" ] || [ "x${line}x" == "xx"  ]; then
> +      if [ "x${letter}x" = "x#x" ] || [ "x${line}x" = "xx"  ]; then

Rather than wasting a $() and gawk subprocess to compute $letter, you
can use the much more efficient:

case $line in
  '#'*) # line started with #
    ;;
  *)    # line did not start with #
    ;;
esac

> @@ -327,13 +320,13 @@ function main() {
>    fi
>  
>    if [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ]; then
> -    pushd "${PWD}" > /dev/null
> +    curdir="${PWD}"
>      . ./test-lib.sh
>      if [ $? -ne 0 ]; then
>          exit 1
>      fi
>      test_intro $this_test
> -    popd > /dev/null
> +    cd "${curdir}"

You really should check whether cd succeeds; as failure to return to the
parent directory deserves aborting the script to avoid corrupting
unintended files in a different directory.

> Index: libvirt-tck/scripts/networks/100-apply-verify-host.t
> ===================================================================
> --- libvirt-tck.orig/scripts/networks/100-apply-verify-host.t
> +++ libvirt-tck/scripts/networks/100-apply-verify-host.t
> @@ -1,10 +1,5 @@
> -#!/bin/bash
> +#!/bin/sh
>  
>  pwd=$(dirname $0)
>  
> -pushd ${PWD} > /dev/null
> -
> -cd ${pwd}
> -bash ./networkApplyTest.sh --tap-test
> -
> -popd > /dev/null
> +(cd ${pwd}; sh ./networkApplyTest.sh --tap-test)

To be robust to starting the script via a relative pathname that starts
with -, I'd write this as:

pwd=$(dirname -- "$0")
(cd -- "${pwd}" && sh ./networkApplyTest.sh --tap-test

> Index: libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat
> ===================================================================
> --- libvirt-tck.orig/scripts/networks/networkxml2hostout/tck-testnet-1.dat
> +++ libvirt-tck/scripts/networks/networkxml2hostout/tck-testnet-1.dat
> @@ -5,7 +5,7 @@ MASQUERADE  all  --  10.1.2.0/24        
>  #iptables -n -L FORWARD | grep ' 10\\.1\\.2\\.'
>  ACCEPT     all  --  0.0.0.0/0            10.1.2.0/24         state RELATED,ESTABLISHED 
>  ACCEPT     all  --  10.1.2.0/24          0.0.0.0/0           
> -#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*\\(dnsmasq[[:print:]*]\\)|\\1|p'
> +#ps aux | sed -n '/dnsmasq .*10\\.1\\.2\\./ s|.*dnsmasq|dnsmasq|p'

Hmm; you used 'read' to read in these lines, but unless you use 'read
-r', that means that backslash interpolation is taking place in the
shell.  'read -r' is portable to POSIX (and therefore dash), even if it
is not portable to generic /bin/sh (but we already have lots of other
things that are not portable to generic /bin/sh that aren't worth
fretting about, since we can assume that the nwfilter tests only make
sense on Linux where /bin/sh will mostly conform to POSIX).  Maybe it's
better to fix the driver scripts to use 'read -r', at which point these
lines could use single \, while at the same time working around the
problem you were seeing where dash interpolated \1 inside "" into char(1).

-- 
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]