Hello,
I've been perusing some of the ksh scripts within /usr/sbin/ and noticed some
differences in coding style and shell syntax usage.
Is there an "official" OpenBSD ksh style guide or a list of recommendations?
For example, syspatch, sysupgrade and sysmerge all use double square brackets
'[[' exclusively and the '((' arithmetic operator where appropriate.
Conversely, /usr/sbin/rcctl seems to use double brackets only for pattern
matching/comparisons, and uses the single square bracket '[' almost exclusively.
Is there a specific reason for this? Are there external requirements/goals that
I'm oblivious to?
To test the waters, I've included a diff below that brings rcctl's usage of
comparison operators in line with that of the other shell scripts in /usr/sbin/.
This diff should apply cleanly against current; I grabbed my copy of rcctl.sh
off of the github mirror a few hours ago.
Does this diff look reasonable? Or should I stop tinkering? These changes
passed my rudimentary testing and also got a clean bill of health from
shellcheck.
Regards,
Jordan
--- a/rcctl.sh Sat May 1 12:07:06 2021
+++ b/rcctl.sh Sat May 1 16:33:30 2021
@@ -40,7 +40,7 @@
needs_root()
{
- [ "$(id -u)" -ne 0 ] && _rc_err "${0##*/}: \"$*\" needs root privileges"
+ (($(id -u) != 0)) && _rc_err "${0##*/}: \"$*\" needs root privileges"
}
rcctl_err()
@@ -55,17 +55,17 @@
cd /etc/rc.d && set -- *
for _s; do
[[ ${_s} == +([[:alnum:]_]) ]] || continue
- [ ! -d "${_s}" ] && echo "${_s}"
+ [[ ! -d ${_s} ]] && echo "${_s}"
done
}
pkg_scripts_append()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
rcconf_edit_begin
- if [ -z "${pkg_scripts}" ]; then
+ if [[ -z ${pkg_scripts} ]]; then
echo pkg_scripts="${_svc}" >>${_TMP_RCCONF}
elif ! echo ${pkg_scripts} | grep -qw -- ${_svc}; then
grep -v "^pkg_scripts.*=" /etc/rc.conf.local >${_TMP_RCCONF}
@@ -77,7 +77,7 @@
pkg_scripts_order()
{
local _svcs="$*"
- [ -n "${_svcs}" ] || return
+ [[ -n ${_svcs} ]] || return
needs_root ${action}
local _pkg_scripts _svc
@@ -99,9 +99,9 @@
pkg_scripts_rm()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
- [ -z "${pkg_scripts}" ] && return
+ [[ -z ${pkg_scripts} ]] && return
rcconf_edit_begin
sed "/^pkg_scripts[[:>:]]/{s/[[:<:]]${_svc}[[:>:]]//g
@@ -129,7 +129,7 @@
rcctl_err "cannot modify ${_TMP_RCCONF}"
cat ${_TMP_RCCONF} >/etc/rc.conf.local || \
rcctl_err "cannot append to /etc/rc.conf.local"
- if [ ! -s /etc/rc.conf.local ]; then
+ if [[ ! -s /etc/rc.conf.local ]]; then
rm /etc/rc.conf.local || \
rcctl_err "cannot remove /etc/rc.conf.local"
fi
@@ -142,19 +142,19 @@
local _svc=$1
_rc_check_name "${_svc}" || return
- [ -x "/etc/rc.d/${_svc}" ] && return
+ [[ -x /etc/rc.d/${_svc} ]] && return
svc_is_special ${_svc}
}
svc_is_base()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
local _cached _ret
_cached=$(eval echo \${cached_svc_is_base_${_svc}})
- [ "${_cached}" ] && return "${_cached}"
+ [[ -n ${_cached} ]] && return "${_cached}"
grep -qw "^${_svc}_flags" /etc/rc.conf
_ret=$?
@@ -166,14 +166,14 @@
svc_is_meta()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
local _cached _ret
_cached=$(eval echo \${cached_svc_is_meta_${_svc}})
- [ "${_cached}" ] && return "${_cached}"
+ [[ -n ${_cached} ]] && return "${_cached}"
- [ -r "/etc/rc.d/${_svc}" ] && ! grep -qw "^rc_cmd" /etc/rc.d/${_svc}
+ [[ -r /etc/rc.d/${_svc} ]] && ! grep -qw "^rc_cmd" /etc/rc.d/${_svc}
_ret=$?
set -A cached_svc_is_meta_${_svc} -- ${_ret}
@@ -183,12 +183,12 @@
svc_is_special()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
local _cached _ret
_cached=$(eval echo \${cached_svc_is_special_${_svc}})
- [ "${_cached}" ] && return "${_cached}"
+ [[ -n ${_cached} ]] && return "${_cached}"
echo ${_special_svcs} | grep -qw -- ${_svc}
_ret=$?
@@ -200,7 +200,7 @@
svc_ls()
{
local _lsarg=$1
- [ -n "${_lsarg}" ] || return
+ [[ -n ${_lsarg} ]] || return
# we do not want to return the "status" nor the rc.d(8) script retcode
local _ret=0 _on _svc _started
@@ -222,8 +222,8 @@
off|on)
for _svc in $(svc_ls all); do
svc_get ${_svc} status && _on=1
- [ "${_lsarg}" = "on" -a -n "${_on}" ] || \
- [ "${_lsarg}" = "off" -a -z "${_on}" ] && \
+ [[ ${_lsarg} == on && -n ${_on} ]] || \
+ [[ ${_lsarg} == off && -z ${_on} ]] && \
echo ${_svc}
unset _on
done
@@ -231,8 +231,8 @@
started|stopped)
for _svc in $(ls_rcscripts); do
/etc/rc.d/${_svc} check >/dev/null && _started=1
- [ "${_lsarg}" = "started" -a -n "${_started}" ] || \
- [ "${_lsarg}" = "stopped" -a -z "${_started}" ] && \
+ [[ ${_lsarg} == started && -n ${_started} ]] || \
+ [[ ${_lsarg} == stopped && -z ${_started} ]] && \
echo ${_svc}
unset _started
done
@@ -248,7 +248,7 @@
svc_get()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
local _status=0 _val _var=$2
local daemon_class daemon_flags daemon_logger daemon_rtable
@@ -266,51 +266,51 @@
if ! svc_is_meta ${_svc}; then
# these are expensive, make sure they are explicitly requested
- if [ -z "${_var}" -o "${_var}" = "class" ]; then
+ if [[ -z ${_var} || ${_var} == class ]]; then
getcap -f /etc/login.conf ${_svc} 1>/dev/null 2>&1 && \
daemon_class=${_svc}
- [ -z "${daemon_class}" ] && \
+ [[ -z ${daemon_class} ]] && \
daemon_class="$(svc_getdef ${_svc} class)"
fi
if [[ -z ${_var} || ${_var} == @(flags|status) ]]; then
- [ -z "${daemon_flags}" ] && \
+ [[ -z ${daemon_flags} ]] && \
daemon_flags="$(eval echo \"\${${_svc}_flags}\")"
- [ -z "${daemon_flags}" ] && \
+ [[ -z ${daemon_flags} ]] && \
daemon_flags="$(svc_getdef ${_svc} flags)"
fi
- if [ -z "${_var}" -o "${_var}" = "logger" ]; then
- [ -z "${daemon_logger}" ] && \
+ if [[ -z ${_var} || ${_var} == logger ]]; then
+ [[ -z ${daemon_logger} ]] && \
daemon_logger="$(eval echo \"\${${_svc}_logger}\")"
- [ -z "${daemon_logger}" ] && \
+ [[ -z ${daemon_logger} ]] && \
daemon_logger="$(svc_getdef ${_svc} logger)"
fi
- if [ -z "${_var}" -o "${_var}" = "rtable" ]; then
- [ -z "${daemon_rtable}" ] && \
+ if [[ -z ${_var} || ${_var} == rtable ]]; then
+ [[ -z ${daemon_rtable} ]] && \
daemon_rtable="$(eval echo \"\${${_svc}_rtable}\")"
- [ -z "${daemon_rtable}" ] && \
+ [[ -z ${daemon_rtable} ]] && \
daemon_rtable="$(svc_getdef ${_svc} rtable)"
fi
- if [ -z "${_var}" -o "${_var}" = "timeout" ]; then
- [ -z "${daemon_timeout}" ] && \
+ if [[ -z ${_var} || ${_var} == timeout ]]; then
+ [[ -z ${daemon_timeout} ]] && \
daemon_timeout="$(eval echo \"\${${_svc}_timeout}\")"
- [ -z "${daemon_timeout}" ] && \
+ [[ -z ${daemon_timeout} ]] && \
daemon_timeout="$(svc_getdef ${_svc} timeout)"
fi
- if [ -z "${_var}" -o "${_var}" = "user" ]; then
- [ -z "${daemon_user}" ] && \
+ if [[ -z ${_var} || ${_var} == user ]]; then
+ [[ -z ${daemon_user} ]] && \
daemon_user="$(eval echo \"\${${_svc}_user}\")"
- [ -z "${daemon_user}" ] && \
+ [[ -z ${daemon_user} ]] && \
daemon_user="$(svc_getdef ${_svc} user)"
fi
fi
fi
- [ "${daemon_flags}" = "NO" ] && _status=1
+ [[ ${daemon_flags} == NO ]] && _status=1
- if [ -n "${_var}" ]; then
- [ "${_var}" = "status" ] && return ${_status}
+ if [[ -n ${_var} ]]; then
+ [[ ${_var} == status ]] && return ${_status}
eval _val=\${daemon_${_var}}
- [ -z "${_val}" ] || print -r -- "${_val}"
+ [[ -z ${_val} ]] || print -r -- "${_val}"
else
svc_is_meta ${_svc} && return ${_status}
if svc_is_special ${_svc}; then
@@ -331,7 +331,7 @@
svc_getdef()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
local _status=0 _val _var=$2
local daemon_class daemon_flags daemon_logger daemon_rtable
@@ -341,7 +341,7 @@
# unconditionally parse: we always output flags and/or status
_rc_parse_conf /etc/rc.conf
daemon_flags="$(eval echo \${${_svc}})"
- [ "${daemon_flags}" = "NO" ] && _status=1
+ [[ ${daemon_flags} = NO ]] && _status=1
else
if ! svc_is_base ${_svc}; then
_status=1 # all pkg_scripts are off by default
@@ -351,7 +351,7 @@
# we'll get our default flags from the rc.d script
[[ -z ${_var} || ${_var} == status ]] && \
_rc_parse_conf /etc/rc.conf
- [ "$(eval echo \${${_svc}_flags})" = "NO" ] && _status=1
+ [[ $(eval echo \${${_svc}_flags}) == NO ]] && _status=1
fi
if ! svc_is_meta ${_svc}; then
@@ -359,16 +359,16 @@
. /etc/rc.d/${_svc} >/dev/null 2>&1
daemon_class=daemon
- [ -z "${daemon_rtable}" ] && daemon_rtable=0
- [ -z "${daemon_timeout}" ] && daemon_timeout=30
- [ -z "${daemon_user}" ] && daemon_user=root
+ [[ -z ${daemon_rtable} ]] && daemon_rtable=0
+ [[ -z ${daemon_timeout} ]] && daemon_timeout=30
+ [[ -z ${daemon_user} ]] && daemon_user=root
fi
fi
- if [ -n "${_var}" ]; then
- [ "${_var}" = "status" ] && return ${_status}
+ if [[ -n ${_var} ]]; then
+ [[ ${_var} == status ]] && return ${_status}
eval _val=\${daemon_${_var}}
- [ -z "${_val}" ] || print -r -- "${_val}"
+ [[ -z ${_val} ]] || print -r -- "${_val}"
else
svc_is_meta ${_svc} && return ${_status}
if svc_is_special ${_svc}; then
@@ -388,7 +388,7 @@
svc_rm()
{
local _svc=$1
- [ -n "${_svc}" ] || return
+ [[ -n ${_svc} ]] || return
rcconf_edit_begin
if svc_is_special ${_svc}; then
@@ -407,7 +407,7 @@
svc_set()
{
local _svc=$1 _var=$2
- [ -n "${_svc}" -a -n "${_var}" ] || return
+ [[ -n ${_svc} && -n ${_var} ]] || return
shift 2
local _args="$*"
@@ -415,16 +415,16 @@
# don't check if we are already enabled or disabled because rc.conf(8)
# defaults may have changed in which case we may have a matching
# redundant entry in rc.conf.local that we can drop
- if [ "${_var}" = "status" ]; then
- if [ "${_args}" = "on" ]; then
+ if [[ ${_var} == status ]]; then
+ if [[ ${_args} == on ]]; then
_var="flags"
# keep our flags if we're already enabled
eval "_args=\"\${${_svc}_${_var}}\""
- [ "${_args}" = "NO" ] && unset _args
+ [[ ${_args} == NO ]] && unset _args
if ! svc_is_base ${_svc} && ! svc_is_special ${_svc}; then
pkg_scripts_append ${_svc}
fi
- elif [ "${_args}" = "off" ]; then
+ elif [[ ${_args} == off ]]; then
if ! svc_is_base ${_svc} && ! svc_is_special ${_svc}; then
pkg_scripts_rm ${_svc}
fi
@@ -439,7 +439,7 @@
fi
if svc_is_special ${_svc}; then
- [ "${_var}" = "flags" ] || return
+ [[ ${_var} == flags ]] || return
rcconf_edit_begin
grep -v "^${_svc}.*=" /etc/rc.conf.local >${_TMP_RCCONF}
( svc_getdef ${_svc} status ) || \
@@ -448,38 +448,38 @@
return
fi
- if [ -n "${_args}" ]; then
- if [ "${_var}" = "logger" ]; then
+ if [[ -n ${_args} ]]; then
+ if [[ ${_var} == logger ]]; then
logger -p "${_args}" </dev/null >/dev/null 2>&1 ||
rcctl_err "unknown priority name: \"${_args}\""
fi
- if [ "${_var}" = "rtable" ]; then
+ if [[ ${_var} == rtable ]]; then
[[ ${_args} != +([[:digit:]]) || ${_args} -lt 0 ]] && \
rcctl_err "\"${_args}\" is not an integer"
fi
- if [ "${_var}" = "timeout" ]; then
+ if [[ ${_var} == timeout ]]; then
[[ ${_args} != +([[:digit:]]) || ${_args} -le 0 ]] && \
rcctl_err "\"${_args}\" is not a positive integer"
fi
- if [ "${_var}" = "user" ]; then
+ if [[ ${_var} == user ]]; then
getent passwd "${_args}" >/dev/null || \
rcctl_err "user \"${_args}\" does not exist"
fi
# unset flags if they match the default enabled ones
- [ "${_args}" = "$(svc_getdef ${_svc} ${_var})" ] && \
+ [[ ${_args} == "$(svc_getdef ${_svc} ${_var})" ]] && \
unset _args
fi
# protect leading whitespace
- [ "${_args}" = "${_args# }" ] || _args="\"${_args}\""
+ [[ ${_args} == "${_args# }" ]] || _args="\"${_args}\""
# reset: value may have changed
unset ${_svc}_${_var}
rcconf_edit_begin
grep -v "^${_svc}_${_var}.*=" /etc/rc.conf.local >${_TMP_RCCONF}
- if [ -n "${_args}" ] || \
- ( svc_is_base ${_svc} && ! svc_getdef ${_svc} status && [ "${_var}" ==
"flags" ] ); then
+ if [[ -n ${_args} ]] || \
+ ( svc_is_base ${_svc} && ! svc_getdef ${_svc} status && [[ ${_var} ==
flags ]] ); then
echo "${_svc}_${_var}=${_args}" >>${_TMP_RCCONF}
fi
rcconf_edit_end
@@ -494,7 +494,7 @@
esac
done
shift $((OPTIND-1))
-[ $# -gt 0 ] || usage
+(($# > 0)) || usage
action=$1
ret=0
@@ -515,9 +515,9 @@
disable|enable|start|stop|restart|reload|check)
shift 1
svcs="$*"
- [ -z "${svcs}" ] && usage
+ [[ -z ${svcs} ]] && usage
# it's ok to disable a non-existing daemon
- if [ "${action}" != "disable" ]; then
+ if [[ ${action} != disable ]]; then
for svc in ${svcs}; do
svc_is_avail ${svc} || \
rcctl_err "service ${svc} does not exist" 2
@@ -527,14 +527,14 @@
get|getdef)
svc=$2
var=$3
- [ -z "${svc}" ] && usage
- [ "${svc}" = "all" ] || svc_is_avail ${svc} || \
+ [[ -z ${svc} ]] && usage
+ [[ ${svc} == all ]] || svc_is_avail ${svc} || \
rcctl_err "service ${svc} does not exist" 2
- if [ -n "${var}" ]; then
- [ "${svc}" = "all" ] && usage
+ if [[ -n ${var} ]]; then
+ [[ ${svc} == all ]] && usage
[[ ${var} != @(class|flags|logger|rtable|status|timeout|user) ]]
&& usage
if svc_is_meta ${svc}; then
- [ "${var}" != "status" ] && \
+ [[ ${var} != status ]] && \
rcctl_err "/etc/rc.d/${svc} is a meta script, cannot
\"${action} ${var}\""
fi
if svc_is_special ${svc}; then
@@ -546,18 +546,18 @@
set)
svc=$2
var=$3
- [ $# -ge 3 ] && shift 3 || shift $#
+ (($# >= 3)) && shift 3 || shift $#
args="$*"
- [ -z "${svc}" ] && usage
+ [[ -z ${svc} ]] && usage
# it's ok to disable a non-existing daemon
- if [ "${action} ${var} ${args}" != "set status off" ]; then
+ if [[ "${action} ${var} ${args}" != "set status off" ]]; then
svc_is_avail ${svc} || \
rcctl_err "service ${svc} does not exist" 2
fi
[[ ${var} != @(class|flags|logger|rtable|status|timeout|user) ]] &&
usage
- svc_is_meta ${svc} && [ "${var}" != "status" ] && \
+ svc_is_meta ${svc} && [[ ${var} != status ]] && \
rcctl_err "/etc/rc.d/${svc} is a meta script, cannot \"${action}
${var}\""
- [[ ${var} = flags && ${args} = NO ]] && \
+ [[ ${var} == flags && ${args} == NO ]] && \
rcctl_err "\"flags NO\" contradicts \"${action}\""
if svc_is_special ${svc}; then
[[ ${var} != status ]] && \
@@ -587,7 +587,7 @@
exit ${ret}
;;
get|getdef)
- if [ "${svc}" = "all" ]; then
+ if [[ ${svc} == all ]]; then
for svc in $(svc_ls all); do
( svc_${action} ${svc} "${var}" )
done
@@ -602,7 +602,7 @@
svc_ls ${lsarg}
;;
order)
- if [ -n "${svcs}" ]; then
+ if [[ -n ${svcs} ]]; then
needs_root ${action}
pkg_scripts_order ${svcs}
else
