Turn shellcheck into a normal make target. Fix new files it caught

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 <[email protected]>
Signed-off-by: Ahelenia Ziemiańska <[email protected]>
Closes #10512
Closes #12101

13 files changed, 76 insertions, 38 deletions

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 <<EOT
-$("${ZFS}" list -H -oname -tsnapshot -r "${fs}")
-EOT
+	# shellcheck disable=SC2046
+	IFS="
+" set -- $("${ZFS}" list -H -oname -tsnapshot -r "${fs}")
 
-	echo "%s" "  Snap nr [1-$((i-1))]? " > /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
 
 	# ------------