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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list