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

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



On 11/15/2010 12:40 PM, Stefan Berger wrote:
> v2:
>     - following Eric's suggestions from review of v1
>     - runs with ksh shell as well
> 
> -          [ $((flags & FLAG_LIBVIRT_TEST)) -ne 0 ] && \
> -              test_result $((passctr+failctr)) "" 1
> -          [ $((flags & FLAG_TAP_TEST)) -ne 0 ] && \
> -             tap_fail $((passctr+failctr)) "${xmlfile} : ${cmd}"
> +          [ $(($flags & $FLAG_LIBVIRT_TEST)) -ne 0 ] && \
> +            test_result $(($passctr + $failctr)) "" 1
> +          [ $(($flags & FLAG_TAP_TEST)) -ne 0 ] && \

Missed one.  This should be $FLAG_TAP_TEST

>      test_intro $this_test
> -    popd > /dev/null
> +    cd "${curdir}"
> +    [ $? -ne 0 ] && echo "cd failed" && exit 1

Here, you want to exit unconditionally if cd failed.

cd "${curdir}" || { echo "cd failed" >&2; exit 1; }

>      skip=0
>      if [ "x${skipregex}x" != "xx" ]; then
> -    	skip=`echo ${cmd} | grep -c -E ${skipregex}`
> +    	skip=`printf %s\\n ${cmd} | grep -c -E ${skipregex}`

As long as you're touching this line, prefer $() over ``.

> @@ -486,34 +486,32 @@ function deleteTestFilter() {
>  }
>  
>  
> -function main() {
> -  local prgname="$0"
> -  local vm1 vm2
> -  local xmldir="nwfilterxml2xmlin"
> -  local fwalldir="nwfilterxml2fwallout"
> -  local found=0 vms res
> -  local filtername="tck-testcase"
> -  local libvirtdpid=-1
> -  local flags OPWD
> +main() {
> +  prgname="$0"

Using $0 fails inside of some zsh versions (where it used to name the
function, rather than being global to the shell script); but that's
probably irrelevant to this exercise (I don't know of any Linux versions
that use zsh as /bin/sh, although it has happened on some BSD variants;
and modern zsh has fixed that bug).  At any rate, if it bothers you, the
workaround is to copy $0 to a global variable at the beginning of the
script, rather than assigning prgname inside of every function that
cares about the global state of $0.

At any rate, given your testing with dash and ksh, and the minor nature
of my nits, feel free to push one you've fixed those without needing to
send a v3.

ACK.

-- 
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]