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

9 files changed, 60 insertions, 34 deletions

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