qemu-block
[Top][All Lists]
Advanced

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

Re: [Qemu-block] [PATCH] tests: Avoid non-portable 'echo -ARG'


From: Edgar E. Iglesias
Subject: Re: [Qemu-block] [PATCH] tests: Avoid non-portable 'echo -ARG'
Date: Wed, 28 Jun 2017 16:46:57 +0200
User-agent: Mutt/1.5.24 (2015-08-30)

On Wed, Jun 28, 2017 at 09:21:37AM -0500, Eric Blake wrote:
> POSIX says that backslashes in the arguments to 'echo', as well as
> any use of 'echo -n' and 'echo -e', are non-portable; it recommends
> people should favor 'printf' instead.  This is definitely true where
> we do not control which shell is running (such as in makefile snippets
> or in documentation examples).  But even for scripts where we
> require bash (and therefore, where echo does what we want by default),
> it is still possible to use 'shopt -s xpg_echo' to change bash's
> behavior of echo.  And setting a good example never hurts when we are
> not sure if a snippet will be copied from a bash-only script to a
> general shell script (although I don't change the use of non-portable
> \e for ESC when we know the running shell is bash).
> 
> Replace 'echo -n "..."' with 'printf "..."', and 'echo -e "..."'
> with 'printf "...\n"'.
> 
> In the qemu-iotests check script, also fix unusual shell quoting
> that would result in word-splitting if 'date' outputs a space.

Reviewed-by: Edgar E. Iglesias <address@hidden>



> 
> Signed-off-by: Eric Blake <address@hidden>
> ---
> 
> Of course, Stefan's pending patch:
> [PATCH 3/5] qemu-iotests: 068: extract _qemu() function
> also touches 068, so there may be some (obvious) merge conflicts
> to resolve there depending on what goes in first.
> 
>  qemu-options.hx             |  4 ++--
>  tests/multiboot/run_test.sh | 10 +++++-----
>  tests/qemu-iotests/051      |  7 ++++---
>  tests/qemu-iotests/068      |  2 +-
>  tests/qemu-iotests/142      | 48 
> ++++++++++++++++++++++-----------------------
>  tests/qemu-iotests/171      | 14 ++++++-------
>  tests/qemu-iotests/check    | 18 ++++++++---------
>  tests/rocker/all            | 10 +++++-----
>  tests/tcg/cris/Makefile     |  8 ++++----
>  9 files changed, 61 insertions(+), 60 deletions(-)
> 
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 896ff17..c8205bb 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -4351,7 +4351,7 @@ The simplest (insecure) usage is to provide the secret 
> inline
> 
>  The simplest secure usage is to provide the secret via a file
> 
> - # echo -n "letmein" > mypasswd.txt
> + # printf "letmein" > mypasswd.txt
>   # $QEMU -object secret,id=sec0,file=mypasswd.txt,format=raw
> 
>  For greater security, AES-256-CBC should be used. To illustrate usage,
> @@ -4379,7 +4379,7 @@ telling openssl to base64 encode the result, but it 
> could be left
>  as raw bytes if desired.
> 
>  @example
> - # SECRET=$(echo -n "letmein" |
> + # SECRET=$(printf "letmein" |
>              openssl enc -aes-256-cbc -a -K $KEY -iv $IV)
>  @end example
> 
> diff --git a/tests/multiboot/run_test.sh b/tests/multiboot/run_test.sh
> index 78d7edf..35bfe0e 100755
> --- a/tests/multiboot/run_test.sh
> +++ b/tests/multiboot/run_test.sh
> @@ -26,7 +26,7 @@ run_qemu() {
>      local kernel=$1
>      shift
> 
> -    echo -e "\n\n=== Running test case: $kernel $@ ===\n" >> test.log
> +    printf "\n\n=== Running test case: $kernel $@ ===\n\n" >> test.log
> 
>      $QEMU \
>          -kernel $kernel \
> @@ -68,21 +68,21 @@ for t in mmap modules; do
>      pass=1
> 
>      if [ $debugexit != 1 ]; then
> -        echo -e "\e[31m ?? \e[0m $t (no debugexit used, exit code $ret)"
> +        printf "\e[31m ?? \e[0m $t (no debugexit used, exit code $ret)\n"
>          pass=0
>      elif [ $ret != 0 ]; then
> -        echo -e "\e[31mFAIL\e[0m $t (exit code $ret)"
> +        printf "\e[31mFAIL\e[0m $t (exit code $ret)\n"
>          pass=0
>      fi
> 
>      if ! diff $t.out test.log > /dev/null 2>&1; then
> -        echo -e "\e[31mFAIL\e[0m $t (output difference)"
> +        printf "\e[31mFAIL\e[0m $t (output difference)\n"
>          diff -u $t.out test.log
>          pass=0
>      fi
> 
>      if [ $pass == 1 ]; then
> -        echo -e "\e[32mPASS\e[0m $t"
> +        printf "\e[32mPASS\e[0m $t\n"
>      fi
> 
>  done
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 26c29de..322c4a8 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -217,7 +217,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
>  # Test 142 checks the direct=on cases
> 
>  for cache in writeback writethrough unsafe invalid_value; do
> -    echo -e "info block\ninfo block file\ninfo block backing\ninfo block 
> backing-file" | \
> +    printf "info block\ninfo block file\ninfo block backing\ninfo block 
> backing-file\n" | \
>      run_qemu -drive 
> file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id
>  -nodefaults
>  done
> 
> @@ -325,8 +325,9 @@ echo "qemu-io $device_id \"write -P 0x22 0 4k\"" | 
> run_qemu -drive file="$TEST_I
> 
>  $QEMU_IO -c "read -P 0x22 0 4k" "$TEST_IMG" | _filter_qemu_io
> 
> -echo -e "qemu-io $device_id \"write -P 0x33 0 4k\"\ncommit $device_id" | 
> run_qemu -drive file="$TEST_IMG",snapshot=on,if=none,id=$device_id\
> -                                                                       | 
> _filter_qemu_io
> +printf "qemu-io $device_id \"write -P 0x33 0 4k\"\ncommit $device_id\n" |
> +    run_qemu -drive file="$TEST_IMG",snapshot=on,if=none,id=$device_id |
> +    _filter_qemu_io
> 
>  $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io
> 
> diff --git a/tests/qemu-iotests/068 b/tests/qemu-iotests/068
> index 3801b65..0d48b6c 100755
> --- a/tests/qemu-iotests/068
> +++ b/tests/qemu-iotests/068
> @@ -76,7 +76,7 @@ for extra_args in \
>      _make_test_img $IMG_SIZE
> 
>      # Give qemu some time to boot before saving the VM state
> -    bash -c 'sleep 1; echo -e "savevm 0\nquit"' | _qemu $extra_args
> +    bash -c 'sleep 1; printf "savevm 0\nquit\n"' | _qemu $extra_args
>      # Now try to continue from that VM state (this should just work)
>      echo quit | _qemu $extra_args -loadvm 0
>  done
> diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
> index 9a5b713..1639c83 100755
> --- a/tests/qemu-iotests/142
> +++ b/tests/qemu-iotests/142
> @@ -94,36 +94,36 @@ function check_cache_all()
>      # cache.direct is supposed to be inherited by both bs->file and
>      # bs->backing
> 
> -    echo -e "cache.direct=on on none0"
> +    printf "cache.direct=on on none0\n"
>      echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.direct=on | 
> grep -e "Cache" -e "[Cc]annot|[Cc]ould not|[Cc]an't"
> -    echo -e "\ncache.direct=on on file"
> +    printf "\ncache.direct=on on file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.direct=on 
> | grep -e "Cache" -e "[Cc]annot|[Cc]ould not|[Cc]an't"
> -    echo -e "\ncache.direct=on on backing"
> +    printf "\ncache.direct=on on backing\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",backing.cache.direct=on | grep -e "Cache" -e 
> "[Cc]annot|[Cc]ould not|[Cc]an't"
> -    echo -e "\ncache.direct=on on backing-file"
> +    printf "\ncache.direct=on on backing-file\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",backing.file.cache.direct=on | grep -e "Cache" -e 
> "[Cc]annot|[Cc]ould not|[Cc]an't"
> 
>      # cache.writeback is supposed to be inherited by bs->backing; bs->file
>      # always gets cache.writeback=on
> 
> -    echo -e "\n\ncache.writeback=off on none0"
> +    printf "\n\ncache.writeback=off on none0\n"
>      echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | 
> grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.writeback=off on file"
> +    printf "\ncache.writeback=off on file\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",file.cache.writeback=off | grep -e "doesn't" -e "does not"
> -    echo -e "\ncache.writeback=off on backing"
> +    printf "\ncache.writeback=off on backing\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",backing.cache.writeback=off | grep -e "doesn't" -e "does not"
> -    echo -e "\ncache.writeback=off on backing-file"
> +    printf "\ncache.writeback=off on backing-file\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",backing.file.cache.writeback=off | grep -e "doesn't" -e "does 
> not"
> 
>      # cache.no-flush is supposed to be inherited by both bs->file and 
> bs->backing
> 
> -    echo -e "\n\ncache.no-flush=on on none0"
> +    printf "\n\ncache.no-flush=on on none0\n"
>      echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.no-flush=on | 
> grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.no-flush=on on file"
> +    printf "\ncache.no-flush=on on file\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",file.cache.no-flush=on | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.no-flush=on on backing"
> +    printf "\ncache.no-flush=on on backing\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",backing.cache.no-flush=on | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.no-flush=on on backing-file"
> +    printf "\ncache.no-flush=on on backing-file\n"
>      echo "$hmp_cmds" | run_qemu -drive 
> "$files","$ids",backing.file.cache.no-flush=on | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
>  }
> 
> @@ -236,35 +236,35 @@ function check_cache_all_separate()
>  {
>      # Check cache.direct
> 
> -    echo -e "cache.direct=on on blk"
> +    printf "cache.direct=on on blk\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive 
> "$drv_file" -drive "$drv_img",cache.direct=on | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.direct=on on file"
> +    printf "\ncache.direct=on on file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive 
> "$drv_file",cache.direct=on -drive "$drv_img" | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.direct=on on backing"
> +    printf "\ncache.direct=on on backing\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive 
> "$drv_bk",cache.direct=on -drive "$drv_file" -drive "$drv_img" | grep -e 
> "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.direct=on on backing-file"
> +    printf "\ncache.direct=on on backing-file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.direct=on -drive 
> "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> 
>      # Check cache.writeback
> 
> -    echo -e "\n\ncache.writeback=off on blk"
> +    printf "\n\ncache.writeback=off on blk\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive 
> "$drv_file" -drive "$drv_img",cache.writeback=off | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.writeback=off on file"
> +    printf "\ncache.writeback=off on file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive 
> "$drv_file",cache.writeback=off -drive "$drv_img" | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.writeback=off on backing"
> +    printf "\ncache.writeback=off on backing\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive 
> "$drv_bk",cache.writeback=off -drive "$drv_file" -drive "$drv_img" | grep -e 
> "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.writeback=off on backing-file"
> +    printf "\ncache.writeback=off on backing-file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.writeback=off 
> -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> 
>      # Check cache.no-flush
> 
> -    echo -e "\n\ncache.no-flush=on on blk"
> +    printf "\n\ncache.no-flush=on on blk\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive 
> "$drv_file" -drive "$drv_img",cache.no-flush=on | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.no-flush=on on file"
> +    printf "\ncache.no-flush=on on file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive "$drv_bk" -drive 
> "$drv_file",cache.no-flush=on -drive "$drv_img" | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.no-flush=on on backing"
> +    printf "\ncache.no-flush=on on backing\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile" -drive 
> "$drv_bk",cache.no-flush=on -drive "$drv_file" -drive "$drv_img" | grep -e 
> "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
> -    echo -e "\ncache.no-flush=on on backing-file"
> +    printf "\ncache.no-flush=on on backing-file\n"
>      echo "$hmp_cmds" | run_qemu -drive "$drv_bkfile",cache.no-flush=on 
> -drive "$drv_bk" -drive "$drv_file" -drive "$drv_img" | grep -e "Cache" -e 
> "[Cc]annot\|[Cc]ould not\|[Cc]an't"
>  }
> 
> diff --git a/tests/qemu-iotests/171 b/tests/qemu-iotests/171
> index 257be10..40c67c8 100755
> --- a/tests/qemu-iotests/171
> +++ b/tests/qemu-iotests/171
> @@ -45,15 +45,15 @@ _supported_os Linux
> 
>  # Create JSON with options
>  img_json() {
> -    echo -n 'json:{"driver":"raw", '
> -    echo -n "\"offset\":\"$img_offset\", "
> +    printf 'json:{"driver":"raw", '
> +    printf "\"offset\":\"$img_offset\", "
>      if [ "$img_size" -ne -1 ] ; then
> -        echo -n "\"size\":\"$img_size\", "
> +        printf "\"size\":\"$img_size\", "
>      fi
> -    echo -n '"file": {'
> -    echo -n    '"driver":"file", '
> -    echo -n    "\"filename\":\"$TEST_IMG\" "
> -    echo -n "} }"
> +    printf '"file": {'
> +    printf    '"driver":"file", '
> +    printf    "\"filename\":\"$TEST_IMG\" "
> +    printf "} }"
>  }
> 
>  do_general_test() {
> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 4b1c674..0a13df9 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check
> @@ -141,7 +141,7 @@ _wallclock()
>  _timestamp()
>  {
>      now=`date "+%T"`
> -    echo -n " [$now]"
> +    printf " [$now]"
>  }
> 
>  _wrapup()
> @@ -255,7 +255,7 @@ seq="check"
>  for seq in $list
>  do
>      err=false
> -    echo -n "$seq"
> +    printf "$seq"
>      if [ -n "$TESTS_REMAINING_LOG" ] ; then
>          sed -e "s/$seq//" -e 's/  / /' -e 's/^ *//' $TESTS_REMAINING_LOG > 
> $TESTS_REMAINING_LOG.tmp
>          mv $TESTS_REMAINING_LOG.tmp $TESTS_REMAINING_LOG
> @@ -281,9 +281,9 @@ do
>          rm -f $seq.out.bad
>          lasttime=`sed -n -e "/^$seq /s/.* //p" <$TIMESTAMP_FILE`
>          if [ "X$lasttime" != X ]; then
> -                echo -n " ${lasttime}s ..."
> +                printf " ${lasttime}s ..."
>          else
> -                echo -n "        "        # prettier output with timestamps.
> +                printf "        "        # prettier output with timestamps.
>          fi
>          rm -f core $seq.notrun
> 
> @@ -291,7 +291,7 @@ do
>          echo "$seq" > "${TEST_DIR}"/check.sts
> 
>          start=`_wallclock`
> -        $timestamp && echo -n "        ["`date "+%T"`"]"
> +        $timestamp && printf "        [$(date "+%T")]"
> 
>          if [ "$(head -n 1 "$source_iotests/$seq")" == "#!/usr/bin/env 
> python" ]; then
>              run_command="$PYTHON $seq"
> @@ -314,21 +314,21 @@ do
> 
>          if [ -f core ]
>          then
> -            echo -n " [dumped core]"
> +            printf " [dumped core]"
>              mv core $seq.core
>              err=true
>          fi
> 
>          if [ -f $seq.notrun ]
>          then
> -            $timestamp || echo -n " [not run] "
> -            $timestamp && echo " [not run]" && echo -n "        $seq -- "
> +            $timestamp || printf " [not run] "
> +            $timestamp && echo " [not run]" && printf "        $seq -- "
>              cat $seq.notrun
>              notrun="$notrun $seq"
>          else
>              if [ $sts -ne 0 ]
>              then
> -                echo -n " [failed, exit status $sts]"
> +                printf " [failed, exit status $sts]"
>                  err=true
>              fi
> 
> diff --git a/tests/rocker/all b/tests/rocker/all
> index d5ae963..3f9b786 100755
> --- a/tests/rocker/all
> +++ b/tests/rocker/all
> @@ -1,19 +1,19 @@
> -echo -n "Running port test...              "
> +printf "Running port test...              "
>  ./port
>  if [ $? -eq 0 ]; then echo "pass"; else echo "FAILED"; exit 1; fi
> 
> -echo -n "Running bridge test...            "
> +printf "Running bridge test...            "
>  ./bridge
>  if [ $? -eq 0 ]; then echo "pass"; else echo "FAILED"; exit 1; fi
> 
> -echo -n "Running bridge STP test...        "
> +printf "Running bridge STP test...        "
>  ./bridge-stp
>  if [ $? -eq 0 ]; then echo "pass"; else echo "FAILED"; exit 1; fi
> 
> -echo -n "Running bridge VLAN test...       "
> +printf "Running bridge VLAN test...       "
>  ./bridge-vlan
>  if [ $? -eq 0 ]; then echo "pass"; else echo "FAILED"; exit 1; fi
> 
> -echo -n "Running bridge VLAN STP test...   "
> +printf "Running bridge VLAN STP test...   "
>  ./bridge-vlan-stp
>  if [ $? -eq 0 ]; then echo "pass"; else echo "FAILED"; exit 1; fi
> diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
> index 6b3dba4..6888263 100644
> --- a/tests/tcg/cris/Makefile
> +++ b/tests/tcg/cris/Makefile
> @@ -150,17 +150,17 @@ check_addcv17.tst: crtv10.o sysv10.o
>  build: $(CRT) $(SYS) $(TESTCASES)
> 
>  check: $(CRT) $(SYS) $(TESTCASES)
> -     @echo -e "\nQEMU simulator."
> +     @printf "\nQEMU simulator.\n"
>       for case in $(TESTCASES); do \
> -             echo -n "$$case "; \
> +             printf "$$case "; \
>               SIMARGS=; \
>               case $$case in *v17*) SIMARGS="-cpu crisv17";; esac; \
>               $(SIM) $$SIMARGS ./$$case; \
>       done
>  check-g: $(CRT) $(SYS) $(TESTCASES)
> -     @echo -e "\nGDB simulator."
> +     @printf "\nGDB simulator.\n"
>       @for case in $(TESTCASES); do \
> -             echo -n "$$case "; \
> +             printf "$$case "; \
>               $(SIMG) $$case; \
>       done
> 
> -- 
> 2.9.4
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]