From c3ef9f7528d160faa08bbddfa29d7ad58835e1bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=BD=D0=B0=D0=B1?= Date: Fri, 21 May 2021 23:43:38 +0200 Subject: [PATCH] Turn shellcheck into a normal make target. Fix new files it caught MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This checks every file it checked (and a few more), but explicitly instead of "if it works it works" best-effort (which wasn't that good anyway) Reviewed-by: Brian Behlendorf Signed-off-by: Ahelenia Ziemiańska Closes #10512 Closes #12101 --- Makefile.am | 15 ++--- cmd/Makefile.am | 6 ++ cmd/fsck_zfs/Makefile.am | 1 + cmd/vdev_id/Makefile.am | 2 + cmd/zed/Makefile.am | 2 + cmd/zed/zed.d/Makefile.am | 1 + cmd/zpool/Makefile.am | 1 + cmd/zpool/zpool.d/smart | 6 +- cmd/zvol_wait/Makefile.am | 2 + config/Shellcheck.am | 8 +++ config/always-shellcheck.m4 | 7 +++ config/zfs-build.m4 | 1 + contrib/Makefile.am | 4 ++ contrib/bash_completion.d/Makefile.am | 5 ++ contrib/bash_completion.d/zfs.in | 23 +++++--- contrib/bpftrace/Makefile.am | 4 ++ .../dracut/02zfsexpandknowledge/Makefile.am | 1 + contrib/dracut/90zfs/Makefile.am | 1 + contrib/dracut/90zfs/module-setup.sh.in | 2 +- contrib/dracut/Makefile.am | 3 + contrib/initramfs/Makefile.am | 3 + contrib/initramfs/hooks/Makefile.am | 1 + contrib/initramfs/scripts/Makefile.am | 7 ++- .../initramfs/scripts/local-top/Makefile.am | 2 + contrib/initramfs/scripts/zfs | 58 ++++++++++--------- etc/Makefile.am | 4 ++ etc/default/Makefile.am | 4 ++ etc/init.d/Makefile.am | 3 + etc/init.d/zfs-import.in | 5 +- etc/init.d/zfs-mount.in | 36 ++++++------ etc/init.d/zfs-zed.in | 1 + etc/systemd/Makefile.am | 3 + etc/zfs/Makefile.am | 3 + etc/zfs/zfs-functions.in | 35 ++++++----- scripts/Makefile.am | 18 ++++-- scripts/dkms.mkconf | 12 ++-- scripts/dkms.postbuild | 7 ++- scripts/kmodtool | 14 +++-- tests/Makefile.am | 5 ++ .../tests/perf/scripts/prefetch_io.sh | 6 +- 40 files changed, 214 insertions(+), 108 deletions(-) create mode 100644 config/Shellcheck.am create mode 100644 config/always-shellcheck.m4 diff --git a/Makefile.am b/Makefile.am index 689816bdc..4fd3f9b3d 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1,3 +1,5 @@ +include $(top_srcdir)/config/Shellcheck.am + ACLOCAL_AMFLAGS = -I config SUBDIRS = include @@ -123,17 +125,8 @@ cstyle: filter_executable = -exec test -x '{}' \; -print -PHONY += shellcheck -shellcheck: - @if type shellcheck > /dev/null 2>&1; then \ - shellcheck --exclude=SC1090,SC1117,SC1091 --format=gcc \ - $$(find ${top_srcdir} -name "config*" -prune -name tests -prune \ - -o -name "*.sh" -o -name "*.sh.in" -type f) \ - $$(find ${top_srcdir}/cmd/zpool/zpool.d/* \ - -type f ${filter_executable}); \ - else \ - echo "skipping shellcheck because shellcheck is not installed"; \ - fi +SHELLCHECKDIRS = cmd contrib etc scripts tests +SHELLCHECKSCRIPTS = autogen.sh PHONY += checkabi storeabi checkabi: lib diff --git a/cmd/Makefile.am b/cmd/Makefile.am index a3db31679..5fc9e8397 100644 --- a/cmd/Makefile.am +++ b/cmd/Makefile.am @@ -1,3 +1,5 @@ +include $(top_srcdir)/config/Shellcheck.am + SUBDIRS = zfs zpool zdb zhack zinject zstream ztest SUBDIRS += fsck_zfs vdev_id raidz_test zfs_ids_to_path SUBDIRS += zpool_influxdb @@ -5,6 +7,9 @@ SUBDIRS += zpool_influxdb CPPCHECKDIRS = zfs zpool zdb zhack zinject zstream ztest CPPCHECKDIRS += raidz_test zfs_ids_to_path zpool_influxdb +# TODO: #12084: SHELLCHECKDIRS = fsck_zfs vdev_id zpool +SHELLCHECKDIRS = fsck_zfs zpool + if USING_PYTHON SUBDIRS += arcstat arc_summary dbufstat endif @@ -12,6 +17,7 @@ endif if BUILD_LINUX SUBDIRS += mount_zfs zed zgenhostid zvol_id zvol_wait CPPCHECKDIRS += mount_zfs zed zgenhostid zvol_id +SHELLCHECKDIRS += zed endif PHONY = cppcheck diff --git a/cmd/fsck_zfs/Makefile.am b/cmd/fsck_zfs/Makefile.am index 67583ac75..f8139f117 100644 --- a/cmd/fsck_zfs/Makefile.am +++ b/cmd/fsck_zfs/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am dist_sbin_SCRIPTS = fsck.zfs diff --git a/cmd/vdev_id/Makefile.am b/cmd/vdev_id/Makefile.am index fb815faad..4071c6d5e 100644 --- a/cmd/vdev_id/Makefile.am +++ b/cmd/vdev_id/Makefile.am @@ -1 +1,3 @@ +include $(top_srcdir)/config/Shellcheck.am + dist_udev_SCRIPTS = vdev_id diff --git a/cmd/zed/Makefile.am b/cmd/zed/Makefile.am index 884760415..7b662994d 100644 --- a/cmd/zed/Makefile.am +++ b/cmd/zed/Makefile.am @@ -1,8 +1,10 @@ include $(top_srcdir)/config/Rules.am +include $(top_srcdir)/config/Shellcheck.am AM_CFLAGS += $(LIBUDEV_CFLAGS) $(LIBUUID_CFLAGS) SUBDIRS = zed.d +SHELLCHECKDIRS = $(SUBDIRS) sbin_PROGRAMS = zed diff --git a/cmd/zed/zed.d/Makefile.am b/cmd/zed/zed.d/Makefile.am index 8b2d0c200..3eece353e 100644 --- a/cmd/zed/zed.d/Makefile.am +++ b/cmd/zed/zed.d/Makefile.am @@ -1,5 +1,6 @@ include $(top_srcdir)/config/Rules.am include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am EXTRA_DIST += README diff --git a/cmd/zpool/Makefile.am b/cmd/zpool/Makefile.am index 4446cf04a..aad45d4f7 100644 --- a/cmd/zpool/Makefile.am +++ b/cmd/zpool/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Rules.am +include $(top_srcdir)/config/Shellcheck.am AM_CFLAGS += $(LIBBLKID_CFLAGS) $(LIBUUID_CFLAGS) diff --git a/cmd/zpool/zpool.d/smart b/cmd/zpool/zpool.d/smart index f8854b752..b71591abb 100755 --- a/cmd/zpool/zpool.d/smart +++ b/cmd/zpool/zpool.d/smart @@ -53,7 +53,7 @@ get_filename_from_dir() num_files=$(find "$dir" -maxdepth 1 -type f | wc -l) mod=$((pid % num_files)) i=0 - find "$dir" -type f -printf "%f\n" | while read -r file ; do + find "$dir" -type f -printf '%f\n' | while read -r file ; do if [ "$mod" = "$i" ] ; then echo "$file" break @@ -231,11 +231,11 @@ esac with_vals=$(echo "$out" | grep -E "$scripts") if [ -n "$with_vals" ]; then echo "$with_vals" - without_vals=$(echo "$scripts" | tr "|" "\n" | + without_vals=$(echo "$scripts" | tr '|' '\n' | grep -v -E "$(echo "$with_vals" | awk -F "=" '{print $1}')" | awk '{print $0"="}') else - without_vals=$(echo "$scripts" | tr "|" "\n" | awk '{print $0"="}') + without_vals=$(echo "$scripts" | tr '|' '\n' | awk '{print $0"="}') fi if [ -n "$without_vals" ]; then diff --git a/cmd/zvol_wait/Makefile.am b/cmd/zvol_wait/Makefile.am index 564031c97..2e5bf3323 100644 --- a/cmd/zvol_wait/Makefile.am +++ b/cmd/zvol_wait/Makefile.am @@ -1 +1,3 @@ +include $(top_srcdir)/config/Shellcheck.am + dist_bin_SCRIPTS = zvol_wait diff --git a/config/Shellcheck.am b/config/Shellcheck.am new file mode 100644 index 000000000..a22333ac4 --- /dev/null +++ b/config/Shellcheck.am @@ -0,0 +1,8 @@ +.PHONY: shellcheck +shellcheck: $(SCRIPTS) $(SHELLCHECKSCRIPTS) +if HAVE_SHELLCHECK + [ -z "$(SCRIPTS)$(SHELLCHECKSCRIPTS)" ] && exit; shellcheck $$([ -n "$(SHELLCHECK_SHELL)" ] && echo "--shell=$(SHELLCHECK_SHELL)") --exclude=SC1090,SC1091$(SHELLCHECK_IGNORE) --format=gcc $(SCRIPTS) $(SHELLCHECKSCRIPTS) +else + @[ -z "$(SCRIPTS)$(SHELLCHECKSCRIPTS)" ] && exit; echo "skipping shellcheck of" $(SCRIPTS) $(SHELLCHECKSCRIPTS) "because shellcheck is not installed" +endif + @set -e; for dir in $(SHELLCHECKDIRS); do $(MAKE) -C $$dir shellcheck; done diff --git a/config/always-shellcheck.m4 b/config/always-shellcheck.m4 new file mode 100644 index 000000000..1e154e1c5 --- /dev/null +++ b/config/always-shellcheck.m4 @@ -0,0 +1,7 @@ +dnl # +dnl # Check if shellcheck is available. +dnl # +AC_DEFUN([ZFS_AC_CONFIG_ALWAYS_SHELLCHECK], [ + AC_CHECK_PROG([SHELLCHECK], [shellcheck], [yes]) + AM_CONDITIONAL([HAVE_SHELLCHECK], [test "x$SHELLCHECK" = "xyes"]) +]) diff --git a/config/zfs-build.m4 b/config/zfs-build.m4 index cee5c87e7..c4fe07c81 100644 --- a/config/zfs-build.m4 +++ b/config/zfs-build.m4 @@ -207,6 +207,7 @@ AC_DEFUN([ZFS_AC_CONFIG_ALWAYS], [ ZFS_AC_CONFIG_ALWAYS_PYZFS ZFS_AC_CONFIG_ALWAYS_SED ZFS_AC_CONFIG_ALWAYS_CPPCHECK + ZFS_AC_CONFIG_ALWAYS_SHELLCHECK ]) AC_DEFUN([ZFS_AC_CONFIG], [ diff --git a/contrib/Makefile.am b/contrib/Makefile.am index 9547878d0..5ec13ece5 100644 --- a/contrib/Makefile.am +++ b/contrib/Makefile.am @@ -1,3 +1,5 @@ +include $(top_srcdir)/config/Shellcheck.am + SUBDIRS = bash_completion.d pyzfs zcp if BUILD_LINUX SUBDIRS += bpftrace dracut initramfs @@ -6,3 +8,5 @@ if PAM_ZFS_ENABLED SUBDIRS += pam_zfs_key endif DIST_SUBDIRS = bash_completion.d bpftrace dracut initramfs pam_zfs_key pyzfs zcp + +SHELLCHECKDIRS = bash_completion.d bpftrace dracut initramfs diff --git a/contrib/bash_completion.d/Makefile.am b/contrib/bash_completion.d/Makefile.am index 8fbe03688..8c8d1aceb 100644 --- a/contrib/bash_completion.d/Makefile.am +++ b/contrib/bash_completion.d/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am bashcompletiondir = $(sysconfdir)/bash_completion.d @@ -6,3 +7,7 @@ noinst_DATA = zfs EXTRA_DIST += $(noinst_DATA) SUBSTFILES += $(noinst_DATA) + +SHELLCHECKSCRIPTS = $(noinst_DATA) +SHELLCHECK_SHELL = bash +SHELLCHECK_IGNORE = ,SC2207 diff --git a/contrib/bash_completion.d/zfs.in b/contrib/bash_completion.d/zfs.in index f650c8f50..41ce2f871 100644 --- a/contrib/bash_completion.d/zfs.in +++ b/contrib/bash_completion.d/zfs.in @@ -139,6 +139,7 @@ __zfs_match_multiple_snapshots() fi local range_start range_start="$(expr "$cur" : '\(.*%\)')" + # shellcheck disable=SC2016 $__ZFS_CMD list -H -o name -s name -t snapshot -d 1 "$base_dataset" | sed 's$.*@$'"$range_start"'$g' fi else @@ -163,7 +164,7 @@ __zfs_argument_chosen() then return 0 fi - for property in $@ + for property in "$@" do if [[ $prev == "$property"* ]] then @@ -181,6 +182,7 @@ __zfs_complete_ordered_arguments() local list2=$2 local cur=$3 local extra=$4 + # shellcheck disable=SC2086 if __zfs_argument_chosen $list1 then COMPREPLY=($(compgen -W "$list2 $extra" -- "$cur")) @@ -193,9 +195,10 @@ __zfs_complete_multiple_options() { local options=$1 local cur=$2 + local existing_opts COMPREPLY=($(compgen -W "$options" -- "${cur##*,}")) - local existing_opts=$(expr "$cur" : '\(.*,\)') + existing_opts=$(expr "$cur" : '\(.*,\)') if [[ $existing_opts ]] then COMPREPLY=( "${COMPREPLY[@]/#/${existing_opts}}" ) @@ -290,6 +293,7 @@ __zfs_complete() *) if ! __zfs_complete_switch "H,r,p,d,o,t,s" then + # shellcheck disable=SC2046 if __zfs_argument_chosen $(__zfs_get_properties) then COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur")) @@ -303,7 +307,7 @@ __zfs_complete() inherit) if ! __zfs_complete_switch "r" then - __zfs_complete_ordered_arguments "$(__zfs_get_inheritable_properties)" "$(__zfs_match_snapshot)" $cur + __zfs_complete_ordered_arguments "$(__zfs_get_inheritable_properties)" "$(__zfs_match_snapshot)" "$cur" fi ;; list) @@ -369,7 +373,7 @@ __zfs_complete() esac ;; set) - __zfs_complete_ordered_arguments "$(__zfs_get_editable_properties)" "$(__zfs_match_snapshot)" $cur + __zfs_complete_ordered_arguments "$(__zfs_get_editable_properties)" "$(__zfs_match_snapshot)" "$cur" __zfs_complete_nospace ;; upgrade) @@ -388,7 +392,7 @@ __zfs_complete() destroy) if ! __zfs_complete_switch "d,f,n,p,R,r,v" then - __zfs_complete_multiple_options "$(__zfs_match_multiple_snapshots)" $cur + __zfs_complete_multiple_options "$(__zfs_match_multiple_snapshots)" "$cur" __zfs_complete_nospace fi ;; @@ -425,7 +429,7 @@ __zpool_list_pools() __zpool_complete() { - local cur prev cmd cmds + local cur prev cmd cmds pools COMPREPLY=() cur="${COMP_WORDS[COMP_CWORD]}" prev="${COMP_WORDS[COMP_CWORD-1]}" @@ -440,7 +444,7 @@ __zpool_complete() case "${cmd}" in get) - __zfs_complete_ordered_arguments "$(__zpool_get_properties)" "$(__zpool_list_pools)" $cur + __zfs_complete_ordered_arguments "$(__zpool_get_properties)" "$(__zpool_list_pools)" "$cur" return 0 ;; import) @@ -453,12 +457,13 @@ __zpool_complete() return 0 ;; set) - __zfs_complete_ordered_arguments "$(__zpool_get_editable_properties)" "$(__zpool_list_pools)" $cur + __zfs_complete_ordered_arguments "$(__zpool_get_editable_properties)" "$(__zpool_list_pools)" "$cur" __zfs_complete_nospace return 0 ;; add|attach|clear|create|detach|offline|online|remove|replace) - local pools="$(__zpool_list_pools)" + pools="$(__zpool_list_pools)" + # shellcheck disable=SC2086 if __zfs_argument_chosen $pools then _filedir diff --git a/contrib/bpftrace/Makefile.am b/contrib/bpftrace/Makefile.am index 327bc6442..05e4f1c50 100644 --- a/contrib/bpftrace/Makefile.am +++ b/contrib/bpftrace/Makefile.am @@ -1,3 +1,7 @@ +include $(top_srcdir)/config/Shellcheck.am + EXTRA_DIST = \ taskqlatency.bt \ zfs-trace.sh + +SHELLCHECKSCRIPTS = zfs-trace.sh diff --git a/contrib/dracut/02zfsexpandknowledge/Makefile.am b/contrib/dracut/02zfsexpandknowledge/Makefile.am index d31d389a0..b1bbb6bd3 100644 --- a/contrib/dracut/02zfsexpandknowledge/Makefile.am +++ b/contrib/dracut/02zfsexpandknowledge/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am pkgdracutdir = $(dracutdir)/modules.d/02zfsexpandknowledge pkgdracut_SCRIPTS = \ diff --git a/contrib/dracut/90zfs/Makefile.am b/contrib/dracut/90zfs/Makefile.am index a4843827e..19b206ddd 100644 --- a/contrib/dracut/90zfs/Makefile.am +++ b/contrib/dracut/90zfs/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am pkgdracutdir = $(dracutdir)/modules.d/90zfs pkgdracut_SCRIPTS = \ diff --git a/contrib/dracut/90zfs/module-setup.sh.in b/contrib/dracut/90zfs/module-setup.sh.in index b0318de92..6122551c3 100755 --- a/contrib/dracut/90zfs/module-setup.sh.in +++ b/contrib/dracut/90zfs/module-setup.sh.in @@ -60,7 +60,7 @@ install() { # shellcheck disable=SC2050 if [ @LIBFETCH_DYNAMIC@ != 0 ]; then for d in $libdirs; do - [ -e "$d"/@LIBFETCH_SONAME@ ] && dracut_install "$d"/@LIBFETCH_SONAME@ + [ -e "$d/"@LIBFETCH_SONAME@ ] && dracut_install "$d/"@LIBFETCH_SONAME@ done fi dracut_install @mounthelperdir@/mount.zfs diff --git a/contrib/dracut/Makefile.am b/contrib/dracut/Makefile.am index 1065e5e94..8c9a6be08 100644 --- a/contrib/dracut/Makefile.am +++ b/contrib/dracut/Makefile.am @@ -1,3 +1,6 @@ +include $(top_srcdir)/config/Shellcheck.am + SUBDIRS = 02zfsexpandknowledge 90zfs +SHELLCHECKDIRS = $(SUBDIRS) EXTRA_DIST = README.dracut.markdown diff --git a/contrib/initramfs/Makefile.am b/contrib/initramfs/Makefile.am index 5eac6e215..931ceb131 100644 --- a/contrib/initramfs/Makefile.am +++ b/contrib/initramfs/Makefile.am @@ -1,9 +1,12 @@ +include $(top_srcdir)/config/Shellcheck.am + initrddir = /usr/share/initramfs-tools dist_initrd_SCRIPTS = \ zfsunlock SUBDIRS = conf.d conf-hooks.d hooks scripts +SHELLCHECKDIRS = hooks scripts EXTRA_DIST = \ README.initramfs.markdown diff --git a/contrib/initramfs/hooks/Makefile.am b/contrib/initramfs/hooks/Makefile.am index f303e995b..0cd1aafcd 100644 --- a/contrib/initramfs/hooks/Makefile.am +++ b/contrib/initramfs/hooks/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am hooksdir = /usr/share/initramfs-tools/hooks diff --git a/contrib/initramfs/scripts/Makefile.am b/contrib/initramfs/scripts/Makefile.am index 2a142096e..444a5f374 100644 --- a/contrib/initramfs/scripts/Makefile.am +++ b/contrib/initramfs/scripts/Makefile.am @@ -1,6 +1,11 @@ +include $(top_srcdir)/config/Shellcheck.am + scriptsdir = /usr/share/initramfs-tools/scripts -dist_scripts_DATA = \ +dist_scripts_SCRIPTS = \ zfs SUBDIRS = local-top + +SHELLCHECKDIRS = $(SUBDIRS) +SHELLCHECK_SHELL = sh diff --git a/contrib/initramfs/scripts/local-top/Makefile.am b/contrib/initramfs/scripts/local-top/Makefile.am index 1523a907c..897f9b2e2 100644 --- a/contrib/initramfs/scripts/local-top/Makefile.am +++ b/contrib/initramfs/scripts/local-top/Makefile.am @@ -1,3 +1,5 @@ +include $(top_srcdir)/config/Shellcheck.am + localtopdir = /usr/share/initramfs-tools/scripts/local-top dist_localtop_SCRIPTS = \ diff --git a/contrib/initramfs/scripts/zfs b/contrib/initramfs/scripts/zfs index b112ac0d2..38122594b 100644 --- a/contrib/initramfs/scripts/zfs +++ b/contrib/initramfs/scripts/zfs @@ -5,6 +5,8 @@ # # Enable this by passing boot=zfs on the kernel command line. # +# $quiet, $root, $rpool, $bootfs come from the cmdline: +# shellcheck disable=SC2154 # Source the common functions . /etc/zfs/zfs-functions @@ -102,14 +104,10 @@ find_rootfs() # Support function to get a list of all pools, separated with ';' find_pools() { - CMD="$*" - - pools=$($CMD 2> /dev/null | \ + pools=$("$@" 2> /dev/null | \ grep -E "pool:|^[a-zA-Z0-9]" | \ sed 's@.*: @@' | \ - while read -r pool; do \ - printf "%s" "$pool;" - done) + tr '\n' ';') echo "${pools%%;}" # Return without the last ';'. } @@ -203,7 +201,7 @@ import_pool() # exists). if [ -n "$USE_DISK_BY_ID" ] && [ -z "$ZPOOL_IMPORT_PATH" ] then - dirs="$(for dir in $(echo /dev/disk/by-*) + dirs="$(for dir in /dev/disk/by-* do # Ignore by-vdev here - we want it first! echo "$dir" | grep -q /by-vdev && continue @@ -329,6 +327,7 @@ mount_fs() # Need the _original_ datasets mountpoint! mountpoint=$(get_fs_value "$fs" mountpoint) + ZFS_CMD="mount -o zfsutil -t zfs" if [ "$mountpoint" = "legacy" ] || [ "$mountpoint" = "none" ]; then # Can't use the mountpoint property. Might be one of our # clones. Check the 'org.zol:mountpoint' property set in @@ -348,15 +347,11 @@ mount_fs() fi fi + # If it's not a legacy filesystem, it can only be a + # native one... if [ "$mountpoint" = "legacy" ]; then ZFS_CMD="mount -t zfs" - else - # If it's not a legacy filesystem, it can only be a - # native one... - ZFS_CMD="mount -o zfsutil -t zfs" fi - else - ZFS_CMD="mount -o zfsutil -t zfs" fi # Possibly decrypt a filesystem using native encryption. @@ -553,7 +548,6 @@ rollback_snap() ask_user_snap() { fs="$1" - i=1 # We need to temporarily disable debugging. Set 'debug' so we # remember to enabled it again. @@ -566,16 +560,25 @@ ask_user_snap() # Because we need the resulting snapshot, which is sent on # stdout to the caller, we use stderr for our questions. echo "What snapshot do you want to boot from?" > /dev/stderr - while read -r snap; do - echo " $i: ${snap}" > /dev/stderr - eval "$(echo SNAP_$i=$snap)" - i=$((i + 1)) - done < /dev/stderr - read -r snapnr + i=1 + for snap in "$@"; do + echo " $i: $snap" + i=$((i + 1)) + done > /dev/stderr + + # expr instead of test here because [ a -lt 0 ] errors out, + # but expr falls back to lexicographical, which works out right + snapnr=0 + while expr "$snapnr" "<" 1 > /dev/null || + expr "$snapnr" ">" "$#" > /dev/null + do + printf "%s" "Snap nr [1-$#]? " > /dev/stderr + read -r snapnr + done # Re-enable debugging. if [ -n "${debug}" ]; then @@ -583,7 +586,7 @@ EOT set -x fi - echo "$(eval echo '$SNAP_'$snapnr)" + eval echo '$'"$snapnr" } setup_snapshot_booting() @@ -703,7 +706,7 @@ mountroot() # ------------ # Look for the cache file (if any). - [ ! -f ${ZPOOL_CACHE} ] && unset ZPOOL_CACHE + [ ! -f "${ZPOOL_CACHE}" ] && unset ZPOOL_CACHE # ------------ # Compatibility: 'ROOT' is for Debian GNU/Linux (etc), @@ -793,7 +796,8 @@ mountroot() # # Reassign the variable by dumping the environment and # stripping the zfs-bootfs= prefix. Let the shell handle - # quoting through the eval command. + # quoting through the eval command: + # shellcheck disable=SC2046 eval ZFS_RPOOL=$(set | sed -n -e 's,^zfs-bootfs=,,p') fi @@ -947,7 +951,7 @@ mountroot() touch /run/zfs_unlock_complete if [ -e /run/zfs_unlock_complete_notify ]; then - read -r zfs_unlock_complete_notify < /run/zfs_unlock_complete_notify + read -r < /run/zfs_unlock_complete_notify fi # ------------ diff --git a/etc/Makefile.am b/etc/Makefile.am index ac71da944..aa9ff182c 100644 --- a/etc/Makefile.am +++ b/etc/Makefile.am @@ -1,5 +1,9 @@ +include $(top_srcdir)/config/Shellcheck.am + SUBDIRS = zfs sudoers.d +SHELLCHECKDIRS = zfs if BUILD_LINUX +SHELLCHECKDIRS += default $(ZFS_INIT_SYSV) SUBDIRS += default $(ZFS_INIT_SYSTEMD) $(ZFS_INIT_SYSV) $(ZFS_MODULE_LOAD) endif DIST_SUBDIRS = default init.d zfs systemd modules-load.d sudoers.d diff --git a/etc/default/Makefile.am b/etc/default/Makefile.am index 0ec868e13..b88eb5494 100644 --- a/etc/default/Makefile.am +++ b/etc/default/Makefile.am @@ -1,5 +1,9 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am initconf_SCRIPTS = zfs SUBSTFILES += $(initconf_SCRIPTS) + +SHELLCHECK_SHELL = sh +SHELLCHECK_IGNORE = ,SC2034 diff --git a/etc/init.d/Makefile.am b/etc/init.d/Makefile.am index 9285a995a..f93af1fd7 100644 --- a/etc/init.d/Makefile.am +++ b/etc/init.d/Makefile.am @@ -1,7 +1,10 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am EXTRA_DIST += README.md init_SCRIPTS = zfs-import zfs-mount zfs-share zfs-zed SUBSTFILES += $(init_SCRIPTS) + +SHELLCHECK_SHELL = dash # local variables diff --git a/etc/init.d/zfs-import.in b/etc/init.d/zfs-import.in index 6b1b2f243..e4bc7b833 100755 --- a/etc/init.d/zfs-import.in +++ b/etc/init.d/zfs-import.in @@ -72,6 +72,7 @@ do_import_all_visible() local exception dir ZPOOL_IMPORT_PATH RET=0 r=1 # In case not shutdown cleanly. + # shellcheck disable=SC2154 [ -n "$init" ] && rm -f /etc/dfs/sharetab # Just simplify code later on. @@ -157,7 +158,7 @@ do_import_all_visible() echo "$dir" | grep -q /by-vdev && continue [ ! -d "$dir" ] && continue - echo -n "$dir:" + printf "%s" "$dir:" done | sed 's,:$,,g')" if [ -d "/dev/disk/by-vdev" ] @@ -214,6 +215,7 @@ do_import_all_visible() # Import by using ZPOOL_IMPORT_PATH (either set above or in # the config file) _or_ with the 'built in' default search # paths. This is the preferred way. + # shellcheck disable=SC2086 "$ZPOOL" import -N ${ZPOOL_IMPORT_OPTS} "$pool" 2> /dev/null r="$?" ; RET=$((RET + r)) if [ "$r" -eq 0 ] @@ -235,6 +237,7 @@ do_import_all_visible() zfs_log_progress_msg " using cache file" fi + # shellcheck disable=SC2086 "$ZPOOL" import -c "$ZPOOL_CACHE" -N ${ZPOOL_IMPORT_OPTS} \ "$pool" 2> /dev/null r="$?" ; RET=$((RET + r)) diff --git a/etc/init.d/zfs-mount.in b/etc/init.d/zfs-mount.in index cb571faf9..000619b67 100755 --- a/etc/init.d/zfs-mount.in +++ b/etc/init.d/zfs-mount.in @@ -80,11 +80,11 @@ do_mount() read_mtab "^/dev/(zd|zvol)" read_fstab "^/dev/(zd|zvol)" - i=0; var=$(eval echo "FSTAB_$i") - while [ -n "$(eval echo "$""$var")" ] + i=0; var="FSTAB_0" + while [ -n "$(eval echo "\$$var")" ] do - mntpt=$(eval echo "$""$var") - dev=$(eval echo "$"FSTAB_dev_$i) + mntpt=$(eval echo "\$$var") + dev=$(eval echo "\$FSTAB_dev_$i") if ! in_mtab "$mntpt" && ! is_mounted "$mntpt" && [ -e "$dev" ] then check_boolean "$VERBOSE_MOUNT" && \ @@ -93,15 +93,15 @@ do_mount() fi i=$((i + 1)) - var=$(eval echo FSTAB_$i) + var=$(eval echo "FSTAB_$i") done read_mtab "[[:space:]]zfs[[:space:]]" read_fstab "[[:space:]]zfs[[:space:]]" - i=0; var=$(eval echo FSTAB_$i) - while [ -n "$(eval echo "$""$var")" ] + i=0; var=$(eval echo "FSTAB_$i") + while [ -n "$(eval echo "\$$var")" ] do - mntpt=$(eval echo "$""$var") + mntpt=$(eval echo "\$$var") if ! in_mtab "$mntpt" && ! is_mounted "$mntpt" then check_boolean "$VERBOSE_MOUNT" && \ @@ -110,7 +110,7 @@ do_mount() fi i=$((i + 1)) - var=$(eval echo FSTAB_$i) + var=$(eval echo "FSTAB_$i") done check_boolean "$VERBOSE_MOUNT" && zfs_log_end_msg 0 @@ -133,11 +133,11 @@ do_unmount() read_mtab "^/dev/(zd|zvol)" read_fstab "^/dev/(zd|zvol)" - i=0; var=$(eval echo FSTAB_$i) - while [ -n "$(eval echo "$""$var")" ] + i=0; var="FSTAB_0" + while [ -n "$(eval echo "\$$var")" ] do - mntpt=$(eval echo "$""$var") - dev=$(eval echo "$"FSTAB_dev_$i) + mntpt=$(eval echo "\$$var") + dev=$(eval echo "\$FSTAB_dev_$i") if in_mtab "$mntpt" then check_boolean "$VERBOSE_MOUNT" && \ @@ -146,15 +146,15 @@ do_unmount() fi i=$((i + 1)) - var=$(eval echo FSTAB_$i) + var=$(eval echo "FSTAB_$i") done read_mtab "[[:space:]]zfs[[:space:]]" read_fstab "[[:space:]]zfs[[:space:]]" - i=0; var=$(eval echo FSTAB_$i) - while [ -n "$(eval echo "$""$var")" ] + i=0; var="FSTAB_0" + while [ -n "$(eval echo "\$$var")" ] do - mntpt=$(eval echo "$""$var") + mntpt=$(eval echo "\$$var") if in_mtab "$mntpt"; then check_boolean "$VERBOSE_MOUNT" && \ zfs_log_progress_msg "$mntpt " @@ -162,7 +162,7 @@ do_unmount() fi i=$((i + 1)) - var=$(eval echo FSTAB_$i) + var=$(eval echo "FSTAB_$i") done check_boolean "$VERBOSE_MOUNT" && zfs_log_end_msg 0 diff --git a/etc/init.d/zfs-zed.in b/etc/init.d/zfs-zed.in index e5550e500..e5256cbc6 100755 --- a/etc/init.d/zfs-zed.in +++ b/etc/init.d/zfs-zed.in @@ -30,6 +30,7 @@ ZED_NAME="zed" ZED_PIDFILE="@runstatedir@/$ZED_NAME.pid" +# shellcheck disable=SC2034 extra_started_commands="reload" # Exit if the package is not installed diff --git a/etc/systemd/Makefile.am b/etc/systemd/Makefile.am index 7b47b93fc..66232a5ff 100644 --- a/etc/systemd/Makefile.am +++ b/etc/systemd/Makefile.am @@ -1 +1,4 @@ +include $(top_srcdir)/config/Shellcheck.am + SUBDIRS = system system-generators +SHELLCHECKDIRS = system-generators diff --git a/etc/zfs/Makefile.am b/etc/zfs/Makefile.am index b9123c176..3dee81c75 100644 --- a/etc/zfs/Makefile.am +++ b/etc/zfs/Makefile.am @@ -1,4 +1,5 @@ include $(top_srcdir)/config/Substfiles.am +include $(top_srcdir)/config/Shellcheck.am pkgsysconfdir = $(sysconfdir)/zfs @@ -13,3 +14,5 @@ pkgsysconf_SCRIPTS = \ zfs-functions SUBSTFILES += $(pkgsysconf_SCRIPTS) + +SHELLCHECK_SHELL = dash # local variables diff --git a/etc/zfs/zfs-functions.in b/etc/zfs/zfs-functions.in index a07cce60d..2fb065afd 100644 --- a/etc/zfs/zfs-functions.in +++ b/etc/zfs/zfs-functions.in @@ -44,7 +44,7 @@ elif type success > /dev/null 2>&1 ; then fi } - zfs_log_begin_msg() { echo -n "$1 "; } + zfs_log_begin_msg() { printf "%s" "$1 "; } zfs_log_end_msg() { zfs_set_ifs "$OLD_IFS" if [ "$1" -eq 0 ]; then @@ -61,17 +61,17 @@ elif type success > /dev/null 2>&1 ; then echo zfs_set_ifs "$TMP_IFS" } - zfs_log_progress_msg() { echo -n "$""$1"; } + zfs_log_progress_msg() { printf "%s" "$""$1"; } elif type einfo > /dev/null 2>&1 ; then # Gentoo functions zfs_log_begin_msg() { ebegin "$1"; } zfs_log_end_msg() { eend "$1"; } zfs_log_failure_msg() { eend "$1"; } -# zfs_log_progress_msg() { echo -n "$1"; } - zfs_log_progress_msg() { echo -n; } +# zfs_log_progress_msg() { printf "%s" "$1"; } + zfs_log_progress_msg() { :; } else # Unknown - simple substitutes. - zfs_log_begin_msg() { echo -n "$1"; } + zfs_log_begin_msg() { printf "%s" "$1"; } zfs_log_end_msg() { ret=$1 if [ "$ret" -ge 1 ]; then @@ -82,7 +82,7 @@ else return "$ret" } zfs_log_failure_msg() { echo "$1"; } - zfs_log_progress_msg() { echo -n "$1"; } + zfs_log_progress_msg() { printf "%s" "$1"; } fi # Paths to what we need @@ -134,15 +134,15 @@ zfs_daemon_start() { local PIDFILE="$1"; shift local DAEMON_BIN="$1"; shift - local DAEMON_ARGS="$*" if type start-stop-daemon > /dev/null 2>&1 ; then # LSB functions start-stop-daemon --start --quiet --pidfile "$PIDFILE" \ --exec "$DAEMON_BIN" --test > /dev/null || return 1 - start-stop-daemon --start --quiet --exec "$DAEMON_BIN" -- \ - $DAEMON_ARGS || return 2 + # shellcheck disable=SC2086 + start-stop-daemon --start --quiet --exec "$DAEMON_BIN" -- \ + "$@" || return 2 # On Debian, there's a 'sendsigs' script that will # kill basically everything quite early and zed is stopped @@ -153,8 +153,9 @@ zfs_daemon_start() ln -sf "$PIDFILE" /run/sendsigs.omit.d/zed fi elif type daemon > /dev/null 2>&1 ; then - # Fedora/RedHat functions - daemon --pidfile "$PIDFILE" "$DAEMON_BIN" $DAEMON_ARGS + # Fedora/RedHat functions + # shellcheck disable=SC2086 + daemon --pidfile "$PIDFILE" "$DAEMON_BIN" "$@" return $? else # Unsupported @@ -234,7 +235,7 @@ zfs_daemon_reload() return $? elif type killproc > /dev/null 2>&1 ; then # Fedora/RedHat functions - killproc -p "$PIDFILE" "$DAEMON_NAME" -HUP + killproc -p "$PIDFILE" "$DAEMON_NAME" -HUP return $? else # Unsupported @@ -287,6 +288,7 @@ checksystem() # HOWEVER, only do this if we're called at the boot up # (from init), not if we're running interactively (as in # from the shell - we know what we're doing). + # shellcheck disable=SC2154 [ -n "$init" ] && exit 3 fi @@ -301,6 +303,7 @@ checksystem() get_root_pool() { + # shellcheck disable=SC2046 set -- $(mount | grep ' on / ') [ "$5" = "zfs" ] && echo "${1%%/*}" } @@ -338,9 +341,10 @@ load_module() read_mtab() { local match="$1" - local fs mntpnt fstype opts rest TMPFILE + local fs mntpnt fstype opts rest # Unset all MTAB_* variables + # shellcheck disable=SC2046 unset $(env | grep ^MTAB_ | sed 's,=.*,,') while read -r fs mntpnt fstype opts rest; do @@ -352,8 +356,8 @@ read_mtab() # * We need to use the external echo, because the # internal one would interpret the backslash code # (incorrectly), giving us a  instead. - mntpnt=$(/bin/echo "$mntpnt" | sed "s,\\\0,\\\00,g") - fs=$(/bin/echo "$fs" | sed "s,\\\0,\\\00,") + mntpnt=$(/bin/echo "$mntpnt" | sed 's,\\0,\\00,g') + fs=$(/bin/echo "$fs" | sed 's,\\0,\\00,') # Remove 'unwanted' characters. mntpnt=$(printf '%b\n' "$mntpnt" | sed -e 's,/,,g' \ @@ -386,6 +390,7 @@ read_fstab() local i var # Unset all FSTAB_* variables + # shellcheck disable=SC2046 unset $(env | grep ^FSTAB_ | sed 's,=.*,,') i=0 diff --git a/scripts/Makefile.am b/scripts/Makefile.am index f59826fe6..f338cdc39 100644 --- a/scripts/Makefile.am +++ b/scripts/Makefile.am @@ -1,3 +1,5 @@ +include $(top_srcdir)/config/Shellcheck.am + pkgdatadir = $(datadir)/@PACKAGE@ dist_pkgdata_SCRIPTS = \ @@ -7,19 +9,25 @@ dist_pkgdata_SCRIPTS = \ zloop.sh \ zfs-helpers.sh -EXTRA_DIST = \ +EXTRA_SCRIPTS = \ commitcheck.sh \ common.sh.in \ - cstyle.pl \ dkms.mkconf \ dkms.postbuild \ - enum-extract.pl \ kmodtool \ make_gitrev.sh \ man-dates.sh \ - paxcheck.sh \ + paxcheck.sh + +EXTRA_DIST = \ + cstyle.pl \ + enum-extract.pl \ zfs2zol-patch.sed \ - zol2zfs-patch.sed + zol2zfs-patch.sed \ + $(EXTRA_SCRIPTS) + +SHELLCHECK_IGNORE = ,SC1117 +SHELLCHECKSCRIPTS = $(EXTRA_SCRIPTS) define EXTRA_ENVIRONMENT diff --git a/scripts/dkms.mkconf b/scripts/dkms.mkconf index 8649b9318..9d12a8c3b 100755 --- a/scripts/dkms.mkconf +++ b/scripts/dkms.mkconf @@ -6,19 +6,21 @@ pkgcfg=/etc/sysconfig/zfs while getopts "n:v:c:f:" opt; do case $opt in - n) pkgname=$OPTARG ;; - v) pkgver=$OPTARG ;; - c) pkgcfg=$OPTARG ;; + n) pkgname=$OPTARG ;; + v) pkgver=$OPTARG ;; + c) pkgcfg=$OPTARG ;; f) filename=$OPTARG ;; + *) err=1 ;; esac done -if [ -z "${pkgname}" ] || [ -z "${pkgver}" ] || [ -z "${filename}" ]; then +if [ -z "${pkgname}" ] || [ -z "${pkgver}" ] || [ -z "${filename}" ] || + [ -n "${err}" ]; then echo "Usage: $PROG -n -v -c -f " exit 1 fi -cat >${filename} <"${filename}" < -k -n " \ - "-t -v " + "-t -v " exit 1 fi -cp "${tree}/${pkgname}/${pkgver}/build/zfs_config.h" \ +exec cp "${tree}/${pkgname}/${pkgver}/build/zfs_config.h" \ "${tree}/${pkgname}/${pkgver}/build/module/Module.symvers" \ "${tree}/${pkgname}/${pkgver}/${kver}/${arch}/" diff --git a/scripts/kmodtool b/scripts/kmodtool index 35d54bad2..26bacf599 100755 --- a/scripts/kmodtool +++ b/scripts/kmodtool @@ -1,4 +1,5 @@ #!/usr/bin/env bash +# shellcheck disable=SC2086 # kmodtool - Helper script for building kernel module RPMs # Copyright (c) 2003-2012 Ville Skyttä , @@ -38,15 +39,16 @@ prefix= filterfile= target= buildroot= +dashvariant= error_out() { local errorlevel=${1} shift - echo "Error: $@" >&2 + echo "Error: $*" >&2 # the next line is not multi-line safe -- not needed *yet* - echo "%global kmodtool_check echo \"kmodtool error: $@\"; exit ${errorlevel};" - exit ${errorlevel} + echo "%global kmodtool_check echo \"kmodtool error: $*\"; exit ${errorlevel};" + exit "${errorlevel}" } print_rpmtemplate_header() @@ -579,7 +581,7 @@ elif [[ ! "${kmodname}" ]]; then error_out 2 "please pass kmodname with --kmodname" elif [[ ! "${kernels_known_variants}" ]] ; then error_out 2 "could not determine known variants" -elif ( [[ "${obsolete_name}" ]] && [[ ! "${obsolete_version}" ]] ) || ( [[ ! "${obsolete_name}" ]] && [[ "${obsolete_version}" ]] ) ; then +elif { [[ "${obsolete_name}" ]] && [[ ! "${obsolete_version}" ]]; } || { [[ ! "${obsolete_name}" ]] && [[ "${obsolete_version}" ]]; } ; then error_out 2 "you need to provide both --obsolete-name and --obsolete-version" fi @@ -597,7 +599,7 @@ else # we need more sanity checks in this case if [[ ! "${repo}" ]]; then error_out 2 "please provide repo name with --repo" - elif ! $(which buildsys-build-${repo}-kerneldevpkgs &> /dev/null) ; then + elif ! command -v "buildsys-build-${repo}-kerneldevpkgs" &> /dev/null ; then error_out 2 "buildsys-build-${repo}-kerneldevpkgs not found" fi @@ -611,7 +613,7 @@ else kernel_versions_to_build_for="$(buildsys-build-${repo}-kerneldevpkgs --${build_kernels} ${cmdoptions})" returncode=$? - if (( ${returncode} != 0 )); then + if (( returncode != 0 )); then error_out 2 "buildsys-build-${repo}-kerneldevpkgs failed: $(buildsys-build-${repo}-kerneldevpkgs --${build_kernels} ${cmdoptions})" fi diff --git a/tests/Makefile.am b/tests/Makefile.am index f13b19613..4bdde9c45 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1,3 +1,8 @@ +include $(top_srcdir)/config/Shellcheck.am + SUBDIRS = runfiles test-runner zfs-tests EXTRA_DIST = README.md + +SHELLCHECKSCRIPTS = $$(find -name '*.sh') +.PHONY: $(SHELLCHECKSCRIPTS) diff --git a/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh b/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh index d8181f2e8..07688ef21 100755 --- a/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh +++ b/tests/zfs-tests/tests/perf/scripts/prefetch_io.sh @@ -64,17 +64,17 @@ async_upgrade_sync=$(get_async_upgrade_sync) while true do new_prefetch_ios=$(get_prefetch_ios) - printf "%u\n%-24s\t%u\n" "$(date +%s)" "prefetch_ios" \ + printf '%u\n%-24s\t%u\n' "$(date +%s)" "prefetch_ios" \ $(( new_prefetch_ios - prefetch_ios )) prefetch_ios=$new_prefetch_ios new_prefetched_demand_reads=$(get_prefetched_demand_reads) - printf "%-24s\t%u\n" "prefetched_demand_reads" \ + printf '%-24s\t%u\n' "prefetched_demand_reads" \ $(( new_prefetched_demand_reads - prefetched_demand_reads )) prefetched_demand_reads=$new_prefetched_demand_reads new_async_upgrade_sync=$(get_async_upgrade_sync) - printf "%-24s\t%u\n" "async_upgrade_sync" \ + printf '%-24s\t%u\n' "async_upgrade_sync" \ $(( new_async_upgrade_sync - async_upgrade_sync )) async_upgrade_sync=$new_async_upgrade_sync