Pass --enable=all to shellcheck within contrib/

- Remove `SHELLCHECK_IGNORE` in favor of inline suppressions and more general `SHELLCHECK_OPTS`. - Exclude `SC2250` (turned on by `--enable=all`) globally - Pass `--enable=all` to shellcheck for scripts in contrib/: it's very important to catch errors early in areas that are not easily testable. Reviewed-by: George Melikov <[email protected]> Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: szubersk <[email protected]> Closes openzfs#12760
nicman23 · Nov 30, 2021 · 34eef3e · 34eef3e
1 parent 4325de0
commit 34eef3e
Show file tree

Hide file tree

Showing 22 changed files with 65 additions and 50 deletions.
diff --git a/config/Shellcheck.am b/config/Shellcheck.am
@@ -1,7 +1,7 @@
 .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)
+	[ -z "$(SCRIPTS)$(SHELLCHECKSCRIPTS)" ] && exit; shellcheck --format=gcc --exclude=SC1090,SC1091,SC2250 $$([ -n "$(SHELLCHECK_SHELL)" ] && echo "--shell=$(SHELLCHECK_SHELL)") $(SHELLCHECK_OPTS) $(SCRIPTS) $(SHELLCHECKSCRIPTS)
 else
 	@[ -z "$(SCRIPTS)$(SHELLCHECKSCRIPTS)" ] && exit; echo "skipping shellcheck of" $(SCRIPTS) $(SHELLCHECKSCRIPTS) "because shellcheck is not installed"
 endif

diff --git a/contrib/Makefile.am b/contrib/Makefile.am
@@ -10,3 +10,4 @@ endif
 DIST_SUBDIRS = bash_completion.d bpftrace dracut initramfs pam_zfs_key pyzfs zcp
 
 SHELLCHECKDIRS = bash_completion.d bpftrace dracut initramfs
+SHELLCHECK_OPTS = --enable=all
diff --git a/contrib/bash_completion.d/Makefile.am b/contrib/bash_completion.d/Makefile.am
@@ -10,4 +10,4 @@ SUBSTFILES += $(noinst_DATA)
 
 SHELLCHECKSCRIPTS = $(noinst_DATA)
 SHELLCHECK_SHELL = bash
-SHELLCHECK_IGNORE = ,SC2207
+SHELLCHECK_OPTS = --enable=all
diff --git a/contrib/bash_completion.d/zfs.in b/contrib/bash_completion.d/zfs.in
@@ -185,9 +185,9 @@ __zfs_complete_ordered_arguments()
     # shellcheck disable=SC2086
     if __zfs_argument_chosen $list1
     then
-        COMPREPLY=($(compgen -W "$list2 $extra" -- "$cur"))
+        mapfile -t COMPREPLY < <(compgen -W "$list2 $extra" -- "$cur")
     else
-        COMPREPLY=($(compgen -W "$list1 $extra" -- "$cur"))
+        mapfile -t COMPREPLY < <(compgen -W "$list1 $extra" -- "$cur")
     fi
 }
 
@@ -197,9 +197,9 @@ __zfs_complete_multiple_options()
     local cur=$2
     local existing_opts
 
-    COMPREPLY=($(compgen -W "$options" -- "${cur##*,}"))
+    mapfile -t COMPREPLY < <(compgen -W "$options" -- "${cur##*,}")
     existing_opts=$(expr "$cur" : '\(.*,\)')
-    if [[ $existing_opts ]]
+    if [ -n "$existing_opts" ]
     then
         COMPREPLY=( "${COMPREPLY[@]/#/${existing_opts}}" )
     fi
@@ -210,7 +210,7 @@ __zfs_complete_switch()
     local options=$1
     if [[ ${cur:0:1} == - ]]
     then
-        COMPREPLY=($(compgen -W "-{$options}" -- "$cur"))
+        mapfile -t COMPREPLY < <(compgen -W "-{$options}" -- "$cur")
         return 0
     else
         return 1
@@ -244,33 +244,33 @@ __zfs_complete()
     if [[ ${prev##*/} == zfs ]]
     then
         cmds=$(__zfs_get_commands)
-        COMPREPLY=($(compgen -W "$cmds -?" -- "$cur"))
+        mapfile -t COMPREPLY < <(compgen -W "$cmds -?" -- "$cur")
         return 0
     fi
 
     case "${cmd}" in
         bookmark)
             if __zfs_argument_chosen
             then
-                COMPREPLY=($(compgen -W "${prev%@*}# ${prev/@/#}" -- "$cur"))
+                mapfile -t COMPREPLY < <(compgen -W "${prev%@*}# ${prev/@/#}" -- "$cur")
             else
-                COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
             fi
             ;;
         clone)
             case "${prev}" in
                 -o)
-                    COMPREPLY=($(compgen -W "$(__zfs_get_editable_properties)" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "$(__zfs_get_editable_properties)" -- "$cur")
                     __zfs_complete_nospace
                     ;;
                 *)
                     if ! __zfs_complete_switch "o,p"
                     then
                         if __zfs_argument_chosen
                         then
-                            COMPREPLY=($(compgen -W "$(__zfs_list_datasets)" -- "$cur"))
+                            mapfile -t COMPREPLY < <(compgen -W "$(__zfs_list_datasets)" -- "$cur")
                         else
-                            COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                            mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
                         fi
                     fi
                     ;;
@@ -279,7 +279,7 @@ __zfs_complete()
         get)
             case "${prev}" in
                 -d)
-                    COMPREPLY=($(compgen -W "" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "" -- "$cur")
                     ;;
                 -t)
                     __zfs_complete_multiple_options "filesystem volume snapshot bookmark all" "$cur"
@@ -296,7 +296,7 @@ __zfs_complete()
                         # shellcheck disable=SC2046
                         if __zfs_argument_chosen $(__zfs_get_properties)
                         then
-                            COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                            mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
                         else
                             __zfs_complete_multiple_options "$(__zfs_get_properties)" "$cur"
                         fi
@@ -313,7 +313,7 @@ __zfs_complete()
         list)
             case "${prev}" in
                 -d)
-                    COMPREPLY=($(compgen -W "" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "" -- "$cur")
                     ;;
                 -t)
                     __zfs_complete_multiple_options "filesystem volume snapshot bookmark all" "$cur"
@@ -322,51 +322,51 @@ __zfs_complete()
                     __zfs_complete_multiple_options "$(__zfs_get_properties)" "$cur"
                     ;;
                 -s|-S)
-                    COMPREPLY=($(compgen -W "$(__zfs_get_properties)" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "$(__zfs_get_properties)" -- "$cur")
                     ;;
                 *)
                     if ! __zfs_complete_switch "H,r,d,o,t,s,S"
                     then
-                        COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                        mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
                     fi
                     ;;
             esac
             ;;
         promote)
-            COMPREPLY=($(compgen -W "$(__zfs_list_filesystems)" -- "$cur"))
+            mapfile -t COMPREPLY < <(compgen -W "$(__zfs_list_filesystems)" -- "$cur")
             ;;
         rollback)
             if ! __zfs_complete_switch "r,R,f"
             then
-                COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
             fi
             ;;
         send)
             if ! __zfs_complete_switch "D,n,P,p,R,v,e,L,i,I"
             then
                 if __zfs_argument_chosen
                 then
-                    COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
                 else
                     if [[ $prev == -*i* ]]
                     then
-                        COMPREPLY=($(compgen -W "$(__zfs_match_snapshot_or_bookmark)" -- "$cur"))
+                        mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot_or_bookmark)" -- "$cur")
                     else
-                        COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                        mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
                     fi
                 fi
             fi
             ;;
         snapshot)
             case "${prev}" in
                 -o)
-                    COMPREPLY=($(compgen -W "$(__zfs_get_editable_properties)" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "$(__zfs_get_editable_properties)" -- "$cur")
                     __zfs_complete_nospace
                     ;;
                 *)
                     if ! __zfs_complete_switch "o,r"
                     then
-                        COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+                        mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
                         __zfs_complete_nospace
                     fi
                     ;;
@@ -379,12 +379,12 @@ __zfs_complete()
         upgrade)
             case "${prev}" in
                 -a|-V|-v)
-                    COMPREPLY=($(compgen -W "" -- "$cur"))
+                    mapfile -t COMPREPLY < <(compgen -W "" -- "$cur")
                     ;;
                 *)
                     if ! __zfs_complete_switch "a,V,v,r"
                     then
-                        COMPREPLY=($(compgen -W "$(__zfs_list_filesystems)" -- "$cur"))
+                        mapfile -t COMPREPLY < <(compgen -W "$(__zfs_list_filesystems)" -- "$cur")
                     fi
                     ;;
             esac
@@ -397,7 +397,7 @@ __zfs_complete()
             fi
             ;;
         *)
-            COMPREPLY=($(compgen -W "$(__zfs_match_snapshot)" -- "$cur"))
+            mapfile -t COMPREPLY < <(compgen -W "$(__zfs_match_snapshot)" -- "$cur")
             ;;
     esac
     if type __ltrim_colon_completions &> /dev/null
@@ -438,7 +438,7 @@ __zpool_complete()
     if [[ ${prev##*/} == zpool ]]
     then
         cmds=$(__zpool_get_commands)
-        COMPREPLY=($(compgen -W "$cmds" -- "$cur"))
+        mapfile -t COMPREPLY < <(compgen -W "$cmds" -- "$cur")
         return 0
     fi
 
@@ -452,7 +452,7 @@ __zpool_complete()
             then
                 _filedir -d
             else
-                COMPREPLY=($(compgen -W "$(__zpool_list_pools) -d" -- "$cur"))
+                mapfile -t COMPREPLY < <(compgen -W "$(__zpool_list_pools) -d" -- "$cur")
             fi
             return 0
             ;;
@@ -468,12 +468,12 @@ __zpool_complete()
             then
                 _filedir
             else
-                COMPREPLY=($(compgen -W "$pools" -- "$cur"))
+                mapfile -t COMPREPLY < <(compgen -W "$pools" -- "$cur")
             fi
             return 0
             ;;
         *)
-            COMPREPLY=($(compgen -W "$(__zpool_list_pools)" -- "$cur"))
+            mapfile -t COMPREPLY < <(compgen -W "$(__zpool_list_pools)" -- "$cur")
             return 0
             ;;
     esac

diff --git a/contrib/bpftrace/Makefile.am b/contrib/bpftrace/Makefile.am
@@ -5,3 +5,4 @@ EXTRA_DIST = \
 	zfs-trace.sh
 
 SHELLCHECKSCRIPTS = zfs-trace.sh
+SHELLCHECK_OPTS = --enable=all
diff --git a/contrib/dracut/02zfsexpandknowledge/Makefile.am b/contrib/dracut/02zfsexpandknowledge/Makefile.am
@@ -5,4 +5,5 @@ pkgdracutdir = $(dracutdir)/modules.d/02zfsexpandknowledge
 pkgdracut_SCRIPTS = \
 	module-setup.sh
 
+SHELLCHECK_OPTS = --enable=all
 SUBSTFILES += $(pkgdracut_SCRIPTS)
diff --git a/contrib/dracut/90zfs/Makefile.am b/contrib/dracut/90zfs/Makefile.am
@@ -19,6 +19,7 @@ pkgdracut_DATA = \
 	zfs-rollback-bootfs.service
 
 SUBSTFILES += $(pkgdracut_SCRIPTS) $(pkgdracut_DATA)
+SHELLCHECK_OPTS = --enable=all
 
 # Provided by /bin/sleep, and, again, every implementation of that supports this
 CHECKBASHISMS_IGNORE = -e 'sleep only takes one integer' -e 'sleep 0.'
diff --git a/contrib/dracut/90zfs/export-zfs.sh.in b/contrib/dracut/90zfs/export-zfs.sh.in
@@ -20,7 +20,7 @@ _do_zpool_export() {
 		zpool list 2>&1 | vinfo
 	fi
 
-	return ${ret}
+	return "${ret}"
 }
 
 if command -v zpool >/dev/null; then

diff --git a/contrib/dracut/90zfs/import-opts-generator.sh.in b/contrib/dracut/90zfs/import-opts-generator.sh.in
@@ -2,4 +2,5 @@
 
 . /lib/dracut-zfs-lib.sh
 
+# shellcheck disable=SC2154
 echo ZPOOL_IMPORT_OPTS="$ZPOOL_IMPORT_OPTS"
diff --git a/contrib/dracut/90zfs/module-setup.sh.in b/contrib/dracut/90zfs/module-setup.sh.in
@@ -61,9 +61,9 @@ install() {
 		dracut_install /usr/lib*/gcc/**/libgcc_s.so*
 	fi
 	# shellcheck disable=SC2050
-	if [ @LIBFETCH_DYNAMIC@ != 0 ]; then
+	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
@@ -107,7 +107,7 @@ install() {
 		for _service in "zfs-import-scan.service" "zfs-import-cache.service" ; do
 			dracut_install "@systemdunitdir@/$_service"
 			if ! [ -L "${initdir}/$systemdsystemunitdir/zfs-import.target.wants/$_service" ]; then
-				ln -sf ../$_service "${initdir}/$systemdsystemunitdir/zfs-import.target.wants/$_service"
+				ln -sf "../$_service" "${initdir}/$systemdsystemunitdir/zfs-import.target.wants/$_service"
 				type mark_hostonly >/dev/null 2>&1 && mark_hostonly "@systemdunitdir@/$_service"
 			fi
 		done

diff --git a/contrib/dracut/90zfs/parse-zfs.sh.in b/contrib/dracut/90zfs/parse-zfs.sh.in
@@ -49,11 +49,14 @@ case "${root}" in
 
 		info "ZFS: Set ${root} as bootfs."
 		;;
+
+	*)
+		info "ZFS: no ZFS-on-root"
 esac
 
 # Make sure Dracut is happy that we have a root and will wait for ZFS
 # modules to settle before mounting.
-if [ ${wait_for_zfs} -eq 1 ]; then
+if [ "${wait_for_zfs}" -eq 1 ]; then
 	ln -s /dev/null /dev/root 2>/dev/null
 	initqueuedir="${hookdir}/initqueue/finished"
 	test -d "${initqueuedir}" || {

diff --git a/contrib/dracut/90zfs/zfs-lib.sh.in b/contrib/dracut/90zfs/zfs-lib.sh.in
@@ -70,6 +70,7 @@ import_pool() {
 }
 
 _mount_dataset_cb() {
+    # shellcheck disable=SC2154
     mount -o zfsutil -t zfs "${1}" "${NEWROOT}${2}"
 }
 
@@ -91,7 +92,7 @@ mount_dataset() {
         fi
     fi
 
-    return ${ret}
+    return "${ret}"
 }
 
 # for_relevant_root_children DATASET EXEC
@@ -117,7 +118,7 @@ for_relevant_root_children() {
                         ;;
                 esac
             done
-            exit ${_ret}
+            exit "${_ret}"
         )
 }
 
@@ -134,7 +135,7 @@ export_all() {
     done
     IFS="${OLDIFS}"
 
-    return ${ret}
+    return "${ret}"
 }
 
 # ask_for_password
@@ -171,6 +172,7 @@ ask_for_password() {
             --ply-tries) ply_tries="$2"; shift;;
             --tty-tries) tty_tries="$2"; shift;;
             --tty-echo-off) tty_echo_off=yes;;
+            *) echo "ask_for_password(): wrong opt '$1'" >&2;;
         esac
         shift
     done
@@ -202,6 +204,6 @@ ask_for_password() {
         fi
     } 9>/.console_lock
 
-    [ $ret -ne 0 ] && echo "Wrong password" >&2
-    return $ret
+    [ "$ret" -ne 0 ] && echo "Wrong password" >&2
+    return "$ret"
 }
diff --git a/contrib/initramfs/Makefile.am b/contrib/initramfs/Makefile.am
@@ -7,6 +7,7 @@ dist_initrd_SCRIPTS = \
 
 SUBDIRS = conf.d conf-hooks.d hooks scripts
 SHELLCHECKDIRS = hooks scripts
+SHELLCHECK_OPTS = --enable=all
 
 EXTRA_DIST = \
 	README.initramfs.markdown
diff --git a/contrib/initramfs/hooks/Makefile.am b/contrib/initramfs/hooks/Makefile.am
@@ -7,4 +7,5 @@ hooks_SCRIPTS = \
 	zfs \
 	zfsunlock
 
+SHELLCHECK_OPTS = --enable=all
 SUBSTFILES += $(hooks_SCRIPTS)
diff --git a/contrib/initramfs/hooks/zfs.in b/contrib/initramfs/hooks/zfs.in
@@ -30,8 +30,8 @@ find /lib/ -type f -name "libgcc_s.so.[1-9]" | while read -r libgcc; do
 done
 
 # shellcheck disable=SC2050
-if [ @LIBFETCH_DYNAMIC@ != 0 ]; then
-	find /lib/ -name @LIBFETCH_SONAME@ | while read -r libfetch; do
+if [ "@LIBFETCH_DYNAMIC@" != 0 ]; then
+	find /lib/ -name "@LIBFETCH_SONAME@" | while read -r libfetch; do
 		copy_exec "$libfetch"
 	done
 fi

diff --git a/contrib/initramfs/scripts/Makefile.am b/contrib/initramfs/scripts/Makefile.am
@@ -9,3 +9,4 @@ SUBDIRS = local-top
 
 SHELLCHECKDIRS = $(SUBDIRS)
 SHELLCHECK_SHELL = sh
+SHELLCHECK_OPTS = --enable=all
diff --git a/contrib/initramfs/scripts/local-top/Makefile.am b/contrib/initramfs/scripts/local-top/Makefile.am
@@ -4,3 +4,5 @@ localtopdir = /usr/share/initramfs-tools/scripts/local-top
 
 dist_localtop_SCRIPTS = \
         zfs
+
+SHELLCHECK_OPTS = --enable=all
Original file line number	Diff line number	Diff line change
Expand Up		@@ -10,3 +10,4 @@ endif
		DIST_SUBDIRS = bash_completion.d bpftrace dracut initramfs pam_zfs_key pyzfs zcp

		SHELLCHECKDIRS = bash_completion.d bpftrace dracut initramfs
		SHELLCHECK_OPTS = --enable=all
Original file line number	Diff line number	Diff line change
Expand Up		@@ -5,3 +5,4 @@ EXTRA_DIST = \
		zfs-trace.sh

		SHELLCHECKSCRIPTS = zfs-trace.sh
		SHELLCHECK_OPTS = --enable=all
Original file line number	Diff line number	Diff line change
Expand Up		@@ -2,4 +2,5 @@

		. /lib/dracut-zfs-lib.sh

		# shellcheck disable=SC2154
		echo ZPOOL_IMPORT_OPTS="$ZPOOL_IMPORT_OPTS"
Original file line number	Diff line number	Diff line change
Expand Up		@@ -9,3 +9,4 @@ SUBDIRS = local-top

		SHELLCHECKDIRS = $(SUBDIRS)
		SHELLCHECK_SHELL = sh
		SHELLCHECK_OPTS = --enable=all
Original file line number	Diff line number	Diff line change
Expand Up		@@ -4,3 +4,5 @@ localtopdir = /usr/share/initramfs-tools/scripts/local-top

		dist_localtop_SCRIPTS = \
		zfs

		SHELLCHECK_OPTS = --enable=all