Merge branch 'misc/queue' into 'master'
Add initial shellcheck checking
See merge request kvm-unit-tests/kvm-unit-tests!59
diff --git a/.shellcheckrc b/.shellcheckrc
new file mode 100644
index 0000000..491af18
--- /dev/null
+++ b/.shellcheckrc
@@ -0,0 +1,30 @@
+# shellcheck configuration file
+external-sources=true
+
+# Optional extras -- https://www.shellcheck.net/wiki/Optional
+# Possibilities, e.g., -
+# quote‐safe‐variables
+# require-double-brackets
+# require-variable-braces
+# add-default-case
+
+# Disable SC2004 style? I.e.,
+# In run_tests.sh line 67:
+# if (( $unittest_run_queues <= 0 )); then
+# ^------------------^ SC2004 (style): $/${} is unnecessary on arithmetic variables.
+disable=SC2004
+
+# Disable SC2086 for now, double quote to prevent globbing and word
+# splitting. There are lots of places that use it for word splitting
+# (e.g., invoking commands with arguments) that break. Should have a
+# more consistent approach for this (perhaps use arrays for such cases)
+# but for now disable.
+# SC2086 (info): Double quote to prevent globbing and word splitting.
+disable=SC2086
+
+# Disable SC2235. Most developers are used to seeing expressions
+# like a || (b && c), not a || { b && c ; }. The subshell overhead in
+# kvm-unit-tests is negligible as it's not shell-heavy in the first
+# place (time is dominated by qemu startup/shutdown and test execution)
+# SC2235 (style): Use { ..; } instead of (..) to avoid subshell overhead.
+disable=SC2235
diff --git a/Makefile b/Makefile
index 4f35fff..6240d8d 100644
--- a/Makefile
+++ b/Makefile
@@ -141,6 +141,10 @@
-name '*.[chsS]' -exec realpath --relative-base=$(CURDIR) {} \; | sort -u > ./cscope.files
cscope -bk
+.PHONY: shellcheck
+shellcheck:
+ shellcheck -a run_tests.sh */run */efi/run scripts/mkstandalone.sh
+
.PHONY: tags
tags:
ctags -R
diff --git a/README.md b/README.md
index 6e82dc2..2d6f7db 100644
--- a/README.md
+++ b/README.md
@@ -193,3 +193,6 @@
We strive to follow the Linux kernels coding style so it's recommended
to run the kernel's ./scripts/checkpatch.pl on new patches.
+
+Also run `make shellcheck` before submitting a patch which touches bash
+scripts.
diff --git a/configure b/configure
index 49f047c..e13a346 100755
--- a/configure
+++ b/configure
@@ -422,6 +422,8 @@
# create the config
cat <<EOF > config.mak
+# Shellcheck does not see these are used
+# shellcheck disable=SC2034
SRCDIR=$srcdir
PREFIX=$prefix
HOST=$host
diff --git a/run_tests.sh b/run_tests.sh
index 938bb8e..152323f 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -45,6 +45,9 @@
only_tests=""
list_tests=""
args=$(getopt -u -o ag:htj:vl -l all,group:,help,tap13,parallel:,verbose,list,probe-maxsmp -- "$@")
+# Shellcheck likes to test commands directly rather than with $? but sometimes they
+# are too long to put in the same test.
+# shellcheck disable=SC2181
[ $? -ne 0 ] && exit 2;
set -- $args;
while [ $# -gt 0 ]; do
diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash
index 2ac7b0b..8643bab 100644
--- a/scripts/arch-run.bash
+++ b/scripts/arch-run.bash
@@ -44,6 +44,8 @@
if [ "$errors" ]; then
sig=$(grep 'terminating on signal' <<<"$errors")
if [ "$sig" ]; then
+ # This is too complex for ${var/search/replace}
+ # shellcheck disable=SC2001
sig=$(sed 's/.*terminating on signal \([0-9][0-9]*\).*/\1/' <<<"$sig")
fi
fi
@@ -150,7 +152,7 @@
return 77
fi
- migcmdline=$@
+ migcmdline=("$@")
trap 'trap - TERM ; kill 0 ; exit 2' INT TERM
trap 'rm -f ${src_out} ${dst_out} ${src_outfifo} ${dst_outfifo} ${dst_incoming} ${src_qmp} ${dst_qmp} ${src_infifo} ${dst_infifo}' RETURN EXIT
@@ -174,16 +176,22 @@
# Holding both ends of the input fifo open prevents opens from
# blocking and readers getting EOF when a writer closes it.
+ # These fds appear to be unused to shellcheck so quieten the warning.
mkfifo ${src_infifo}
mkfifo ${dst_infifo}
+ # shellcheck disable=SC2034
exec {src_infifo_fd}<>${src_infifo}
+ # shellcheck disable=SC2034
exec {dst_infifo_fd}<>${dst_infifo}
- eval "$migcmdline" \
+ "${migcmdline[@]}" \
-chardev socket,id=mon,path=${src_qmp},server=on,wait=off \
-mon chardev=mon,mode=control \
< ${src_infifo} > ${src_outfifo} &
live_pid=$!
+ # Shellcheck complains about useless cat but it is clearer than a
+ # redirect in this case.
+ # shellcheck disable=SC2002
cat ${src_outfifo} | tee ${src_out} | filter_quiet_msgs &
# Start the first destination QEMU machine in advance of the test
@@ -219,11 +227,14 @@
do_migration ()
{
- eval "$migcmdline" \
+ "${migcmdline[@]}" \
-chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \
-mon chardev=mon,mode=control -incoming unix:${dst_incoming} \
< ${dst_infifo} > ${dst_outfifo} &
incoming_pid=$!
+ # Shellcheck complains about useless cat but it is clearer than a
+ # redirect in this case.
+ # shellcheck disable=SC2002
cat ${dst_outfifo} | tee ${dst_out} | filter_quiet_msgs &
# The test must prompt the user to migrate, so wait for the
@@ -357,7 +368,7 @@
qmp=$(mktemp -u -t panic-qmp.XXXXXXXXXX)
# start VM stopped so we don't miss any events
- eval "$@" -chardev socket,id=mon,path=${qmp},server=on,wait=off \
+ "$@" -chardev socket,id=mon,path=${qmp},server=on,wait=off \
-mon chardev=mon,mode=control -S &
panic_event_count=$(qmp_events ${qmp} | jq -c 'select(.event == "GUEST_PANICKED")' | wc -l)
@@ -415,7 +426,8 @@
{
rm -f $KVM_UNIT_TESTS_ENV
if [ "$KVM_UNIT_TESTS_ENV_OLD" ]; then
- export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD"
+ export KVM_UNIT_TESTS_ENV
+ KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD"
else
unset KVM_UNIT_TESTS_ENV
fi
@@ -427,7 +439,8 @@
if [ "$ENVIRON_DEFAULT" = "yes" ]; then
trap_exit_push 'initrd_cleanup'
[ -f "$KVM_UNIT_TESTS_ENV" ] && export KVM_UNIT_TESTS_ENV_OLD="$KVM_UNIT_TESTS_ENV"
- export KVM_UNIT_TESTS_ENV=$(mktemp)
+ export KVM_UNIT_TESTS_ENV
+ KVM_UNIT_TESTS_ENV=$(mktemp)
env_params
env_file
env_errata || return $?
@@ -465,6 +478,8 @@
[ -n "$ACCEL" ] && QEMU_ACCEL=$ACCEL
fi
QEMU_VERSION_STRING="$($qemu -h | head -1)"
+ # Shellcheck does not see QEMU_MAJOR|MINOR|MICRO are used
+ # shellcheck disable=SC2034
IFS='[ .]' read -r _ _ _ QEMU_MAJOR QEMU_MINOR QEMU_MICRO rest <<<"$QEMU_VERSION_STRING"
fi
env_add_params QEMU_ACCEL QEMU_VERSION_STRING QEMU_MAJOR QEMU_MINOR QEMU_MICRO
@@ -570,7 +585,9 @@
trap_exit_push ()
{
- local old_exit=$(trap -p EXIT | sed "s/^[^']*'//;s/'[^']*$//")
+ local old_exit
+
+ old_exit=$(trap -p EXIT | sed "s/^[^']*'//;s/'[^']*$//")
trap -- "$1; $old_exit" EXIT
}
@@ -593,6 +610,8 @@
set_qemu_accelerator ()
{
+ # Shellcheck does not see ACCEL_PROPS is used
+ # shellcheck disable=SC2034
ACCEL_PROPS=${ACCEL#"${ACCEL%%,*}"}
ACCEL=${ACCEL%%,*}
diff --git a/scripts/common.bash b/scripts/common.bash
index b9413d6..5e9ad53 100644
--- a/scripts/common.bash
+++ b/scripts/common.bash
@@ -78,8 +78,11 @@
}
# The current file has to be the only file sourcing the arch helper
-# file
+# file. Shellcheck can't follow this so help it out. There doesn't appear to be a
+# way to specify multiple alternatives, so we will have to rethink this if things
+# get more complicated.
ARCH_FUNC=scripts/${ARCH}/func.bash
if [ -f "${ARCH_FUNC}" ]; then
+# shellcheck source=scripts/s390x/func.bash
source "${ARCH_FUNC}"
fi
diff --git a/scripts/mkstandalone.sh b/scripts/mkstandalone.sh
index 756647f..2318a85 100755
--- a/scripts/mkstandalone.sh
+++ b/scripts/mkstandalone.sh
@@ -65,6 +65,8 @@
fi
temp_file bin "$kernel"
+ # Don't want to expand $bin but print it as-is.
+ # shellcheck disable=SC2016
args[3]='$bin'
(echo "#!/usr/bin/env bash"
diff --git a/scripts/runtime.bash b/scripts/runtime.bash
index e7af9bd..fb7c83a 100644
--- a/scripts/runtime.bash
+++ b/scripts/runtime.bash
@@ -15,7 +15,9 @@
# We assume that QEMU is going to work if it tried to load the kernel
premature_failure()
{
- local log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
+ local log
+
+ log="$(eval "$(get_cmdline _NO_FILE_4Uhere_)" 2>&1)"
echo "$log" | grep "_NO_FILE_4Uhere_" |
grep -q -e "could not \(load\|open\) kernel" -e "error loading" -e "failed to load" &&
@@ -125,6 +127,8 @@
# the check line can contain multiple files to check separated by a space
# but each check parameter needs to be of the form <path>=<value>
if [ "$check" ]; then
+ # There is no globbing or whitespace allowed in check parameters.
+ # shellcheck disable=SC2206
check=($check)
for check_param in "${check[@]}"; do
path=${check_param%%=*}
