[artix-general] [PATCH v3] various shell improvements

Ethan Sommer e5ten.arch at gmail.com
Mon Jan 6 21:08:41 CET 2020


make POSIX sh compatible
    remove array usage
    remove local builtin usage
    remove herestring usage
    remove string indexing usage

use shellcheck directive to specify source file locations

use manual shell option processing instead of getopt

use : to delimit file names in list instead of space like PATH

check command results directly instead of checking $?

make sed usage POSIX compatible and more concise

use case to check variables against multiple strings instead of a series
of ifs

replace incorrect usages of continue with return where appropriate

Signed-off-by: Ethan Sommer <e5ten.arch at gmail.com>
---
 bin/opensysusers.in |  11 +--
 bin/sysusers.in     | 125 ++++++++++++++----------------
 lib/common.sh       | 185 ++++++++++++++++++++------------------------
 3 files changed, 149 insertions(+), 172 deletions(-)

diff --git a/bin/opensysusers.in b/bin/opensysusers.in
index bc7834c..7bb2ce5 100755
--- a/bin/opensysusers.in
+++ b/bin/opensysusers.in
@@ -4,15 +4,16 @@
 #
 # This is an implementation of sysusers.d spec without systemd command
 
-sysusersver=@VERSION@
-
-source @LIBDIR@/opensysusers/common.sh
+# shellcheck source=lib/common.sh
+. @LIBDIR@/opensysusers/common.sh
 
 get_conf_files
 get_conf_paths
 
+IFS=':'
 for file in ${sysusers_d}; do
-	parse_file ${file}
+	IFS="${_IFS}"
+	parse_file "${file}"
 done
 
-exit ${error}
+exit "${error}"
diff --git a/bin/sysusers.in b/bin/sysusers.in
index 2d5b7aa..3900d48 100755
--- a/bin/sysusers.in
+++ b/bin/sysusers.in
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 # Copyright (c) 2018 Chris Cromer
 # Released under the 2-clause BSD license.
 #
@@ -7,118 +7,107 @@
 sysusersver=@VERSION@
 
 usage() {
-	printf "@BINNAME@\n\n"
-
-	printf "@BINNAME@ creates system users and groups, based on the file\n"
-    printf "format and location specified in sysusers.d(5).\n\n"
-
-	printf "Usage: /usr/bin/@BINNAME@ [OPTIONS...] [CONFIGFILE...]\n\n"
-
-	printf "Options:\n"
-	printf "  --root=root               All paths will be prefixed with the\n"
-    printf "                            given alternate root path, including\n"
-    printf "                            config search paths.\n"
-	printf "  --replace=PATH            Don't run check in the package\n"
-	printf "  --inline                  Treat each positional argument as a\n"
-	printf "                            separate configuration line instead of a\n"
-	printf "                            file name.\n"
-	printf "  -h, --help                Print a short help text and exit.\n"
-	printf "  --version                 Print a short version string and exit.\n"
+	printf '%s\n' \
+		'@BINNAME@' '' \
+		'@BINNAME@ creates system users and groups, based on the file' \
+		'format and location specified in sysusers.d(5).' '' \
+		'Usage: /usr/bin/@BINNAME@ [OPTIONS...] [CONFIGFILE...]' '' \
+		'Options:' \
+		'  --root=root               All paths will be prefixed with the' \
+		'                            given alternate root path, including' \
+		'                            config search paths.' \
+		"  --replace=PATH            Don't run check in the package" \
+		'  --inline                  Treat each positional argument as a' \
+		'                            separate configuration line instead of a' \
+		'                            file name.' \
+		'  -h, --help                Print a short help text and exit.' \
+		'  --version                 Print a short version string and exit.'
+	exit "$1"
 }
 
-TEMP=$(getopt -o "h" -l "root:,replace:,inline,help,version" -n "@BINNAME@" -- "$@")
-if [ $? -ne 0 ]; then
-	usage
-	exit 1;
-fi
-
-arguments=()
 replace=''
-version=0
 inline=0
-error=0
 root=''
 replace=''
-eval set -- "${TEMP}"
-unset TEMP
 
-while true; do
-	case "${1}" in
-		-h|--help)
-			usage
-			exit 0;
-			shift;
-			;;
+while :; do
+	case "$1" in
+		-h|--help) usage 0 ;;
+		--root=*) root="${1#--root=}" ;;
 		--root)
-			root="${2}"
-			shift 2;
+			root="$2"
+			shift
 			;;
+		--replace=*) replace="${1#--replace=}" ;;
 		--replace)
-			replace="${2}"
-			shift 2;
-			;;
-		--inline)
-			inline=1
-			shift;
+			replace="$2"
+			shift
 			;;
+		--inline) inline=1 ;;
 		--version)
-			version=1
-			shift;
+			printf '%s\n' "${sysusersver}"
+			exit 0
 			;;
-		--) shift; args=("${@}"); break;;
-		*) break;;
+		--) shift; break ;;
+		-*) usage 1 ;;
+		*) break ;;
 	esac
+	shift
 done
 
-source @LIBDIR@/opensysusers/common.sh
-
-if [ ${version} -eq 1 ]; then
-	echo "${sysusersver}"
-	exit ${error};
-fi
+# shellcheck source=lib/common.sh
+. @LIBDIR@/opensysusers/common.sh
 
-if [ ${#args[@]} -eq 0 ]; then
+if [ "$#" -eq 0 ]; then
 	get_conf_files
 	get_conf_paths
 
+	IFS=':'
 	for file in ${sysusers_d}; do
-		parse_file ${file}
+		IFS="${_IFS}"
+		parse_file "${file}"
 	done
-	exit ${error};
+	exit "${error}"
 fi
 
-if [ ${inline} -eq 0 ]; then
-	for file in "${args[@]}"; do
-		[ ${file} == '--' ] && continue
+if [ "${inline}" -eq 0 ]; then
+	for file in "$@"; do
+		[ "${file}" = '--' ] && continue
+		IFS=':'
 		for dir in ${sysusers_dirs}; do
 			if [ -f "${dir}/${file}" ]; then
 				parse_file "${dir}/${file}"
 				break
 			fi
 		done
+		IFS="${_IFS}"
 	done
-	if [ "${replace}" != '' ]; then
+	if [ -n "${replace}" ]; then
 		get_conf_files
 		get_conf_paths
 
+		IFS=':'
 		for file in ${sysusers_d}; do
-			parse_file ${file}
+			parse_file "${file}"
 		done
+		IFS="${_IFS}"
 	fi
-	exit ${error};
+	exit "${error}"
 else
-	for string in "${args[@]}"; do
+	for string in "$@"; do
 		parse_string "${string}"
 	done
-	if [ "${replace}" != '' ]; then
+	if [ -n "${replace}" ]; then
 		get_conf_files
 		get_conf_paths
 
+		IFS=':'
 		for file in ${sysusers_d}; do
-			parse_file ${file}
+			parse_file "${file}"
 		done
+		IFS="${_IFS}"
 	fi
-	exit ${error};
+	exit "${error}"
 fi
 
-exit ${error};
+exit "${error}"
diff --git a/lib/common.sh b/lib/common.sh
index 0e84054..0522ac9 100755
--- a/lib/common.sh
+++ b/lib/common.sh
@@ -1,4 +1,4 @@
-#!/bin/bash
+#!/bin/sh
 # Copyright (c) 2018 Chris Cromer
 # Copyright (c) 2012 Gentoo Foundation
 # Released under the 2-clause BSD license.
@@ -6,142 +6,121 @@
 # Common functions and variables needed by opensysusers
 
 die() {
-	echo ${@} 1>&2;
-	exit 1;
+	printf '%s\n' "$*" >&2
+	exit 1
 }
 
 warninvalid() {
-	local msg=$1
-	[ -z "${msg}" ] && msg='ignoring invalid entry'
-	printf "sysusers: ${msg} on line %d of \`%s'\n" "${line}" "${file}"
-	error=$(( error+1 ))
+	[ -z "$1" ] && set -- 'ignoring invalid entry'
+	printf "sysusers: %s on line %d of \`%s'\n" "$1" "${line}" "${file}"
+	: "$((error += 1))"
 } >&2
 
 in_array() {
-	local element
-	for element in "${@:2}"; do
-		[[ "${element}" == "${1}" ]] && return 0;
+	needle="$1"
+	shift
+	for element in "$@"; do
+		[ "${element}" = "${needle}" ] && return 0
 	done
 	return 1
 }
 
 add_group() {
-	local name=${1} id=${2}
-	getent group "${name}" >/dev/null
-	if [ "$?" -ne 0 ]; then
-		if [ "${id}" == '-' ]; then
-			groupadd -r "${name}"
-		else
-			if ! grep -qiw "${id}" /etc/group; then
-				groupadd -g "${id}" "${name}"
-			fi
+	# add_group <name> <id>
+	if ! getent group "$1" >/dev/null; then
+		if [ "$2" = '-' ]; then
+			groupadd -r "$1"
+		elif ! grep -qiw "$2" /etc/group; then
+			groupadd -g "$2" "$1"
 		fi
 	fi
 }
 
 add_user() {
-	local name=${1} id=${2} gecos=${3} home=${4}
-	getent passwd "${name}" >/dev/null
-	if [ "$?" -ne 0 ]; then
-		if [ "${id}" == '-' ]; then
-			useradd -rc "${gecos}" -g "${name}" -d "${home}" -s '/sbin/nologin' "${name}"
+	# add_user <name> <id> <gecos> <home>
+	if getent passwd "$1" >/dev/null; then
+		if [ "$2" = '-' ]; then
+			useradd -rc "$3" -g "$1" -d "$4" -s '/sbin/nologin' "$1"
 		else
-			useradd -rc "${gecos}" -u "${id}" -g "${name}" -d "${home}" -s '/sbin/nologin' "${name}"
+			useradd -rc "$3" -u "$2" -g "$1" -d "$4" -s '/sbin/nologin' "$1"
 		fi
-		passwd -l "${name}" &>/dev/null
+		passwd -l "$1" >/dev/null 2>&1
 	fi
 }
 
 update_login_defs() {
-	local name=${1} id=${2}
-	[ ! "${name}" == '-' ] && warninvalid && return
-	i=1;
-	IFS='-' read -ra temp <<< "${id}"
-	for part in "${temp[@]}"; do
-		if [ "${i}" -eq 1 ]; then
-			min=${part}
-		fi
-		if [ "${i}" -eq 2 ]; then
-			max=${part}
-		fi
-		if [ "${i}" -eq 3 ]; then
-			warninvalid && continue
-		fi
-		i=$(( i+1 ))
+	# update_login_defs <name> <id>
+	[ "$1" != '-' ] && warninvalid && return
+	i=0
+	IFS='-'
+	for part in $2; do
+		case "$((i += 1))" in
+			1) min="${part}" ;;
+			2) max="${part}" ;;
+			3) warninvalid && return ;;
+		esac
 	done
-	[ ${min} -ge ${max} ] && warninvalid "invalid range" && return
+	IFS="${_IFS}"
+	[ "${min}" -ge "${max}" ] && warninvalid "invalid range" && return
 
 	while read -r NAME VALUE; do
-		[ "${NAME}" == 'SYS_UID_MAX' ] && suid_max=${VALUE}
-		[ "${NAME}" == 'SYS_GID_MAX' ] && sgid_max=${VALUE}
+		case "${NAME}" in
+			SYS_UID_MAX) suid_max="${VALUE}" ;;
+			SYS_GID_MAX) sgid_max="${VALUE}" ;;
+		esac
 	done < /etc/login.defs
-	[[ $min -lt $suid_max ]] && warninvalid "invalid range" && return
-	[[ $min -lt $sgid_max ]] && warninvalid "invalid range" && return
+	[ "${min}" -lt "${suid_max}" ] && warninvalid "invalid range" && return
+	[ "${min}" -lt "${sgid_max}" ] && warninvalid "invalid range" && return
 
-	sed -re "s/^(UID_MIN)([[:space:]]+)(.*)/\1\2${min}/" -i /etc/login.defs
-	sed -re "s/^(UID_MAX)([[:space:]]+)(.*)/\1\2${max}/" -i /etc/login.defs
-	sed -re "s/^(GID_MIN)([[:space:]]+)(.*)/\1\2${min}/" -i /etc/login.defs
-	sed -re "s/^(GID_MAX)([[:space:]]+)(.*)/\1\2${max}/" -i /etc/login.defs
+	sed -e "s/^\([GU]ID_MIN\)\([[:space:]]\+\)\(.*\)/\1\2${min}/" \
+		-e "s/^\([GU]ID_MAX\)\([[:space:]]\+\)\(.*\)/\1\2${max}/" \
+		-i /etc/login.defs
 }
 
 parse_file() {
-	local file="${1}"
-	if [ -f ${file} ]; then
-		while read cline; do
-			[[ "${cline}" =~ ^#.*$ ]] && continue
-			parse_string "${cline}"
-		done < ${file}
+	if [ -f "$1" ]; then
+		line=0
+		while read -r cline; do
+			: "$((line += 1))"
+			case "${cline}" in [!#]*) parse_string "${cline}"; esac
+		done < "$1"
 	fi
 }
 
 parse_string() {
-	local line=0 cline="${1}"
-	[ "${cline:0:1}" == "#" ] && continue
-	eval "set args ${cline}; shift"
+	case "$1" in '#'*) return; esac
+	eval "set -- $1"
 	i=0
-	for part in "${@}"; do
-		if [ $i -eq 0 ]; then
-			type="${part}"
-		fi
-		if [ $i -eq 1 ]; then
-			name="${part}"
-		fi
-		if [ $i -eq 2 ]; then
-			id="${part}"
-		fi
-		if [ $i -eq 3 ]; then
-			gecos="${part}"
-		fi
-		if [ $i -eq 4 ]; then
-			home="${part}"
-		fi
-		i=$(( i+1 ))
+	for part in "$@"; do
+		case "${i}" in
+			0) type="${part}" ;;
+			1) name="${part}" ;;
+			2) id="${part}" ;;
+			3) gecos="${part}" ;;
+			4) home="${part}" ;;
+		esac
+		: "$((i += 1))"
 	done
-	line=$(( line+1 ))
+	: "$((line += 1))"
 
 	case "${type}" in
 		u)
-			[ "${id}" == '65535' ] && warninvalid && continue
-			[ "${id}" == '4294967295' ] && warninvalid && continue
-			[ "${home}" == '-' ] && home="/"
-			[ -z "${home}" ] && home="/"
+			case "${id}" in 65535|4294967295) warninvalid; return; esac
+			[ "${home:--}" = - ] && home='/'
 			add_group "${name}" "${id}"
-			[ "${id}" == '-' ] && id=$(getent group "${name}" | cut -d: -f3)
+			[ "${id}" = '-' ] && id="$(getent group "${name}")" id="${id#*:*:}" id="${id%%:*}"
 			add_user "${name}" "${id}" "${gecos}" "${home}"
 		;;
 		g)
-			[ "${id}" == '65535' ] && warninvalid && continue
-			[ "${id}" == '4294967295' ] && warninvalid && continue
-			[ "${home}" == '-' ] && home="/"
-			[ -z "${home}" ] && home="/"
+			case "${id}" in 65535|4294967295) warninvalid; return; esac
+			[ "${home:--}" = '-' ] && home='/'
 			add_group "${name}" "${id}"
 		;;
 		m)
 			add_group "${name}" '-'
-			getent passwd "${name}" >/dev/null
-			if [ "$?" -ne 0 ]; then
+			if ! getent passwd "${name}" >/dev/null; then
 				useradd -r -g "${id}" -s '/sbin/nologin' "${name}"
-				passwd -l "${name}" &>/dev/null
+				passwd -l "${name}" >/dev/null 2>&1
 			else
 				usermod -a -G "${id}" "${name}"
 			fi
@@ -149,7 +128,7 @@ parse_string() {
 		r)
 			update_login_defs "${name}" "${id}"
 		;;
-		*) warninvalid; continue;;
+		*) warninvalid; return ;;
 	esac
 }
 
@@ -161,31 +140,39 @@ parse_string() {
 # `/run/sysusers.d/foo.conf' will always be read after `/etc/sysusers.d/bar.conf'
 
 get_conf_files() {
+	IFS=':'
 	for dir in ${sysusers_dirs}; do
 		[ -d "${dir}" ] && for file in "${dir}"/*.conf; do
-			[ "${replace}" != '' ] && [ "${dir}" == "$(dirname ${replace})" ] && [ "${file##*/}" == "${replace##*/}" ] && continue
-			[ -f "${file}" ] && sysusers_basenames="${sysusers_basenames}\n${file##*/}"
+			[ -n "${replace}" ] &&
+				[ "${dir}" = "$(dirname "${replace}")" ] &&
+				[ "${file##*/}" = "${replace##*/}" ] &&
+				continue
+			[ -f "${file}" ] && sysusers_basenames="${sysusers_basenames}
+${file##*/}"
 		done
 	done
-	FILES="$(printf "${sysusers_basenames}\n" | sort -u )"
+	FILES="$(printf '%s\n' "${sysusers_basenames}" | sort -u)"
 }
 
 get_conf_paths() {
+	IFS="${_IFS}"
 	for b in ${FILES}; do
-		real_f=''
+		real_f='' IFS=':'
 		for d in ${sysusers_dirs}; do
-			[ "${replace}" != '' ] && [ "${d}" == "$(dirname ${replace})" ] && [ "${b}" == "${replace##*/}" ] && continue
-			f=${d}/${b}
-			[ -f "${f}" ] && real_f="${f}"
+			[ -n "${replace}" ] &&
+				[ "${d}" = "$(dirname "${replace}")" ] &&
+				[ "${b}" = "${replace##*/}" ] && continue
+			[ -f "${d}/${b}" ] && real_f="${d}/${b}"
 		done
-		[ -f "${real_f}" ] && sysusers_d="${sysusers_d} ${real_f}"
+		[ -f "${real_f}" ] && sysusers_d="${sysusers_d}:${real_f}"
 	done
 }
 
+_IFS="${IFS}"
 error=0
 FILES=''
 sysusers_basenames=''
 sysusers_d=''
 replace=''
 
-sysusers_dirs="${root}/usr/lib/sysusers.d ${root}/run/sysusers.d ${root}/etc/sysusers.d"
+sysusers_dirs="${root:=}/usr/lib/sysusers.d:${root}/run/sysusers.d:${root}/etc/sysusers.d"
-- 
2.24.1



More information about the artix-general mailing list