ctdb-tests: Avoid ShellCheck warnings
authorMartin Schwenke <mschwenke@ddn.com>
Sun, 2 Jul 2023 23:32:26 +0000 (09:32 +1000)
committerMartin Schwenke <martins@samba.org>
Wed, 5 Jul 2023 11:18:37 +0000 (11:18 +0000)
These are all trivial, so handle them in bulk.

* Change code to avoid (approximately sorted by frequency):

  SC2004 $/${} is unnecessary on arithmetic variables.
  SC2086 Double quote to prevent globbing and word splitting.
  SC2162 read without -r will mangle backslashes.
  SC2254 Quote expansions in case patterns to match literally rather than as a glob.
  SC2154 (warning): <variable> is referenced but not assigned.
  SC3037 (warning): In POSIX sh, echo flags are undefined.
  SC2016 (info): Expressions don't expand in single quotes, use double quotes for that.
  SC2069 (warning): To redirect stdout+stderr, 2>&1 must be last (or use '{ cmd > file; } 2>&1' to clarify).
  SC2124 (warning): Assigning an array to a string! Assign as array, or use * instead of @ to concatenate.
  SC2166 (warning): Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.
  SC2223 (info): This default assignment may cause DoS due to globbing. Quote it.

* Locally disable checks:

  SC2034 (warning): <variable> appears unused. Verify use (or export if used externally).
  SC2086 (info): Double quote to prevent globbing and word splitting. [once]
  SC2120 (warning): <function> references arguments, but none are ever passed.
  SC2317 (info): Command appears to be unreachable. Check usage (or ignore if invoked indirectly).

While touching reads for SC2162, switch unused variables to "_"
instead of "_x", which seems to be preferred by ShellCheck.

Signed-off-by: Martin Schwenke <mschwenke@ddn.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
12 files changed:
ctdb/tests/UNIT/eventscripts/scripts/05.system.sh
ctdb/tests/UNIT/eventscripts/scripts/10.interface.sh
ctdb/tests/UNIT/eventscripts/scripts/11.natgw.sh
ctdb/tests/UNIT/eventscripts/scripts/13.per_ip_routing.sh
ctdb/tests/UNIT/eventscripts/scripts/48.netbios.sh
ctdb/tests/UNIT/eventscripts/scripts/49.winbind.sh
ctdb/tests/UNIT/eventscripts/scripts/50.samba.sh
ctdb/tests/UNIT/eventscripts/scripts/60.nfs.sh
ctdb/tests/UNIT/eventscripts/scripts/91.lvs.sh
ctdb/tests/UNIT/eventscripts/scripts/debug_locks.sh
ctdb/tests/UNIT/eventscripts/scripts/local.sh
ctdb/tests/UNIT/eventscripts/scripts/statd-callout.sh

index b08134bf7c68db7ea4ca8a6d249756cc2dd20f67..0191e55ab0a242d04794f5465a090fb22b4d0e0f 100644 (file)
@@ -1,16 +1,18 @@
+# shellcheck disable=SC2120
+# Arguments used in testcases
 set_mem_usage()
 {
        _mem_usage="${1:-10}" # Default is 10%
        _swap_usage="${2:-0}" # Default is  0%
 
        _swap_total=5857276
-       _swap_free=$(((100 - $_swap_usage) * $_swap_total / 100))
+       _swap_free=$(((100 - _swap_usage) * _swap_total / 100))
 
        _mem_total=3940712
        _mem_free=225268
        _mem_buffers=146120
-       _mem_cached=$(($_mem_total * (100 - $_mem_usage) / 100 - \
-               $_mem_free - $_mem_buffers))
+       _mem_cached=$((_mem_total * (100 - _mem_usage) / 100 - \
+               _mem_free - _mem_buffers))
 
        export FAKE_PROC_MEMINFO="\
 MemTotal:        ${_mem_total} kB
index e5f44997bb9f0d0e34b4b0b8338e06958f08514c..579f3ee7a9b4c98eb1606b2ac4292914fd631fe8 100644 (file)
@@ -14,7 +14,7 @@ _tcp_connections()
        _cip_prefix="${_cip_base%.*}"
        _cip_suffix="${_cip_base##*.}"
 
-       for _i in $(seq 1 $_count); do
+       for _i in $(seq 1 "$_count"); do
                _cip_last=$((_cip_suffix + _i))
                _cip="${_cip_prefix}.${_cip_last}"
                _cport=$((_cport_base + _i))
index d3f591f4de15cac55515fa1d8afc2c756bed2177..3b19895127916bac51ff8ba7abc2a9ec777c028d 100644 (file)
@@ -12,7 +12,7 @@ setup()
 setup_ctdb_natgw()
 {
        # Read from stdin
-       while read _ip _opts; do
+       while read -r _ip _opts; do
                case "$_opts" in
                leader)
                        export FAKE_CTDB_NATGW_LEADER="$_ip"
@@ -29,7 +29,7 @@ setup_ctdb_natgw()
 
        # Assume all of the nodes are on a /24 network and have IPv4
        # addresses:
-       read _ip <"$natgw_nodes"
+       read -r _ip <"$natgw_nodes"
 
        setup_script_options <<EOF
 CTDB_NATGW_NODES="$natgw_nodes"
index e2e29a6493103cb87a6f39a2a3462702dc65ef35..aac2c3d835d8c75e521bb93f31dc231bd294c6ea 100644 (file)
@@ -2,6 +2,8 @@ setup()
 {
        setup_public_addresses
 
+       # shellcheck disable=SC2034
+       # Used in expected output
        service_name="per_ip_routing"
 
        setup_script_options <<EOF
@@ -32,7 +34,7 @@ create_policy_routing_config()
                                cat >/dev/null
                        }
                fi |
-               while read _dev _ip _bits; do
+               while read -r _dev _ip _bits; do
                        _net=$(ipv4_host_addr_to_net "$_ip" "$_bits")
                        _gw="${_net%.*}.254" # a dumb, calculated default
 
index 5289e8faa5aa592d8666139929e177bb683774b4..6efcd8ada87ee63bab57aabbec889facbe9772af 100644 (file)
@@ -1,5 +1,7 @@
 setup()
 {
+       # shellcheck disable=SC2034
+       # Used in expected output
        service_name="netbios"
 
        if [ "$1" != "down" ]; then
index 1f5a71870fe185587e6c4f7fe13a8441e83c3a12..bbe1de2a8d76b4d98df8e43af15c91affea549eb 100644 (file)
@@ -1,5 +1,7 @@
 setup()
 {
+       # shellcheck disable=SC2034
+       # Used in expected output
        service_name="winbind"
 
        if [ "$1" != "down" ]; then
index 8443434e04f46eafdc9d8a7bca0ca2273a911204..88af69bcd5a1f8e8b541d03e10205d56d24de4e7 100644 (file)
@@ -1,5 +1,7 @@
 setup()
 {
+       # shellcheck disable=SC2034
+       # Used in expected output
        service_name="samba"
 
        if [ "$1" != "down" ]; then
@@ -46,9 +48,11 @@ samba_setup_fake_threads()
        _count=0
        for _pid; do
                [ "$_count" -lt 5 ] || break
-               _t=$(program_stack_trace "smbd" $_pid)
+               _t=$(program_stack_trace "smbd" "$_pid")
                _out="${_out:+${_out}${_nl}}${_t}"
                _count=$((_count + 1))
        done
+       # shellcheck disable=SC2034
+       # Used in expected output
        SAMBA_STACK_TRACES="$_out"
 }
index 1e269d56a66479ba5e777fd9f5a03bd0eb12d483..9c614c76bb7cbb9b75978698f750e6b58a13405e 100644 (file)
@@ -3,6 +3,8 @@ setup()
        setup_public_addresses
        setup_shares
 
+       # shellcheck disable=SC2034
+       # Used in expected output
        service_name="nfs"
 
        if [ -z "$CTDB_NFS_DISTRO_STYLE" ]; then
@@ -116,7 +118,7 @@ nfs_setup_fake_threads()
        nfsd)
                export PROCFS_PATH="${CTDB_TEST_TMP_DIR}/proc"
                _threads="${PROCFS_PATH}/fs/nfsd/threads"
-               mkdir -p $(dirname "$_threads")
+               mkdir -p "$(dirname "$_threads")"
                echo $# >"$_threads"
                export FAKE_NFSD_THREAD_PIDS="$*"
                ;;
@@ -129,7 +131,7 @@ nfs_setup_fake_threads()
 guess_output()
 {
        case "$1" in
-       $CTDB_NFS_CALLOUT\ start\ nlockmgr)
+       "${CTDB_NFS_CALLOUT} start nlockmgr")
                case "$CTDB_NFS_DISTRO_STYLE" in
                sysvinit-redhat)
                        echo "&Starting nfslock: OK"
@@ -144,7 +146,7 @@ EOF
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ start\ nfs)
+       "${CTDB_NFS_CALLOUT} start nfs")
                case "$CTDB_NFS_DISTRO_STYLE" in
                sysvinit-redhat)
                        cat <<EOF
@@ -173,14 +175,14 @@ EOF
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ stop\ mountd)
+       "${CTDB_NFS_CALLOUT} stop mountd")
                case "$CTDB_NFS_DISTRO_STYLE" in
                systemd-*)
                        echo "Stopping nfs-mountd: OK"
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ stop\ rquotad)
+       "${CTDB_NFS_CALLOUT} stop rquotad")
                case "$CTDB_NFS_DISTRO_STYLE" in
                systemd-redhat)
                        echo "Stopping rpc-rquotad: OK"
@@ -194,21 +196,21 @@ EOF
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ stop\ status)
+       "${CTDB_NFS_CALLOUT} stop status")
                case "$CTDB_NFS_DISTRO_STYLE" in
                systemd-*)
                        echo "Stopping rpc-statd: OK"
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ start\ mountd)
+       "${CTDB_NFS_CALLOUT} start mountd")
                case "$CTDB_NFS_DISTRO_STYLE" in
                systemd-*)
                        echo "&Starting nfs-mountd: OK"
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ start\ rquotad)
+       "${CTDB_NFS_CALLOUT} start rquotad")
                case "$CTDB_NFS_DISTRO_STYLE" in
                systemd-redhat)
                        echo "&Starting rpc-rquotad: OK"
@@ -218,7 +220,7 @@ EOF
                        ;;
                esac
                ;;
-       $CTDB_NFS_CALLOUT\ start\ status)
+       "${CTDB_NFS_CALLOUT} start status")
                case "$CTDB_NFS_DISTRO_STYLE" in
                systemd-*)
                        echo "&Starting rpc-statd: OK"
@@ -246,7 +248,7 @@ rpc_set_service_failure_response()
 
        # Default
        ok_null
-       if [ $_numfails -eq 0 ]; then
+       if [ "$_numfails" -eq 0 ]; then
                return
        fi
 
@@ -270,12 +272,16 @@ rpc_set_service_failure_response()
                # Subshell to restrict scope variables...
 
                # Defaults
+               # shellcheck disable=SC2034
+               # Unused, but for completeness, possible future use
                family="tcp"
                version=""
                unhealthy_after=1
                restart_every=0
                service_stop_cmd=""
                service_start_cmd=""
+               # shellcheck disable=SC2034
+               # Unused, but for completeness, possible future use
                service_check_cmd=""
                service_debug_cmd=""
 
@@ -298,8 +304,8 @@ $_rpc_service failed RPC check:
 rpcinfo: RPC: Program not registered
 program $_rpc_service${_ver:+ version }${_ver} is not available"
 
-               if [ $unhealthy_after -gt 0 -a \
-                       $_numfails -ge $unhealthy_after ]; then
+               if [ $unhealthy_after -gt 0 ] &&
+                       [ "$_numfails" -ge $unhealthy_after ]; then
                        _unhealthy=true
                        echo 1 >"$_rc_file"
                        echo "ERROR: ${_rpc_check_out}" >>"$_out"
@@ -309,7 +315,7 @@ program $_rpc_service${_ver:+ version }${_ver} is not available"
                fi
 
                if [ $restart_every -gt 0 ] &&
-                       [ $(($_numfails % $restart_every)) -eq 0 ]; then
+                       [ $((_numfails % restart_every)) -eq 0 ]; then
                        if ! $_unhealthy; then
                                echo "WARNING: ${_rpc_check_out}" >>"$_out"
                        fi
@@ -320,15 +326,15 @@ program $_rpc_service${_ver:+ version }${_ver} is not available"
                        guess_output "$service_stop_cmd" >>"$_out"
 
                        if [ -n "$service_debug_cmd" ]; then
-                               $service_debug_cmd 2>&1 >>"$_out"
+                               $service_debug_cmd >>"$_out" 2>&1
                        fi
 
                        guess_output "$service_start_cmd" >>"$_out"
                fi
        )
 
-       read _rc <"$_rc_file"
-       required_result $_rc <"$_out"
+       read -r _rc <"$_rc_file"
+       required_result "$_rc" <"$_out"
 
        rm -f "$_out" "$_rc_file"
 }
@@ -345,10 +351,10 @@ program_stack_traces()
                _pids="$FAKE_RPC_THREAD_PIDS"
        fi
        for _pid in $_pids; do
-               [ $_count -le $_max ] || break
+               [ $_count -le "$_max" ] || break
 
                program_stack_trace "$_prog" "$_pid"
-               _count=$(($_count + 1))
+               _count=$((_count + 1))
        done
 }
 
@@ -385,10 +391,12 @@ nfs_iterate_test()
                shift
        fi
 
+       # shellcheck disable=SC2154
+       # Variables defined in define_test()
        echo "Running $_repeats iterations of \"$script $event\" $args"
 
        _iterate_failcount=0
-       for _iteration in $(seq 1 $_repeats); do
+       for _iteration in $(seq 1 "$_repeats"); do
                # This is not a numerical comparison because $1 will
                # often not be set.
                if [ "$_iteration" = "$1" ]; then
@@ -406,7 +414,7 @@ EOF
                                   >/dev/null 2>&1 ; then
                                _iterate_failcount=0
                        else
-                               _iterate_failcount=$(($_iterate_failcount + 1))
+                               _iterate_failcount=$((_iterate_failcount + 1))
                        fi
                        rpc_set_service_failure_response \
                                "$_rpc_service" $_iterate_failcount
index 7eb4daa9da45c67262a446303549dddda40eee47..1e307c509fae2bb39e1973616caebc0be070c3ea 100644 (file)
@@ -21,7 +21,7 @@ EOF
 
        # Read from stdin
        _pnn=0
-       while read _ip _opts; do
+       while read -r _ip _opts; do
                case "$_opts" in
                leader)
                        FAKE_CTDB_LVS_LEADER="$_pnn"
@@ -34,7 +34,7 @@ EOF
                        echo "$_ip"
                        ;;
                esac
-               _pnn=$(($_pnn + 1))
+               _pnn=$((_pnn + 1))
        done >"$CTDB_LVS_NODES"
 }
 
index 7fe536040993cf363d93caf0c152d5ef4adbdf8e..b0cd0396bd0fff2dcc4714fcef5826568a537917 100644 (file)
@@ -261,6 +261,8 @@ $_out
 ===== End of debug locks PID=PID =====
 EOF
 
+       # shellcheck disable=SC2154
+       # script_dir and script set in define_test()
        script_test "${script_dir}/${script}" \
                "$_lock_helper_pid" \
                "$_helper_scope" \
index 84d0f355218c36a1f04969a7e61579c4648bf56e..39a2a087a2189e276a1e6954dc915c73f826e5e9 100644 (file)
@@ -191,7 +191,7 @@ shares_missing()
                                    "$_type" "${_i}")
                        _out="${_out}${_out:+${_nl}}${_t}"
                done
-               _n=$(($_n + 1))
+               _n=$((_n + 1))
        done
 
        echo "$_out"
@@ -230,7 +230,7 @@ dump_routes()
        ip rule show
 
        ip rule show |
-               while read _p _x _i _x _t; do
+               while read -r _p _ _i _ _t; do
                        # Remove trailing colon after priority/preference.
                        _p="${_p%:}"
                        # Only remove rules that match our priority/preference.
@@ -252,20 +252,22 @@ ipv4_host_addr_to_net()
        _host_ul=0
        for _o in $(
                export IFS="."
+               # shellcheck disable=SC2086
+               # Intentional word splitting
                echo $_host
        ); do
-               _host_ul=$((($_host_ul << 8) + $_o)) # work around Emacs color bug
+               _host_ul=$(((_host_ul << 8) + _o)) # work around Emacs color bug
        done
 
        # Calculate the mask and apply it.
-       _mask_ul=$((0xffffffff << (32 - $_maskbits)))
-       _net_ul=$(($_host_ul & $_mask_ul))
+       _mask_ul=$((0xffffffff << (32 - _maskbits)))
+       _net_ul=$((_host_ul & _mask_ul))
 
        # Now convert to a network address one byte at a time.
        _net=""
        for _o in $(seq 1 4); do
-               _net="$(($_net_ul & 255))${_net:+.}${_net}"
-               _net_ul=$(($_net_ul >> 8))
+               _net="$((_net_ul & 255))${_net:+.}${_net}"
+               _net_ul=$((_net_ul >> 8))
        done
 
        echo "${_net}/${_maskbits}"
@@ -275,6 +277,8 @@ ipv4_host_addr_to_net()
 
 # CTDB fakery
 
+# shellcheck disable=SC2120
+# Argument can be used in testcases
 setup_numnodes()
 {
        export FAKE_CTDB_NUMNODES="${1:-3}"
@@ -326,7 +330,7 @@ ctdb_get_interfaces()
 ctdb_get_1_interface()
 {
        _t=$(ctdb_get_interfaces)
-       echo ${_t%% *}
+       echo "${_t%% *}"
 }
 
 # Print public addresses on this node as: interface IP maskbits
@@ -334,13 +338,13 @@ ctdb_get_1_interface()
 ctdb_get_my_public_addresses()
 {
        ctdb ip -v -X | {
-               read _x # skip header line
+               read -r _ # skip header line
 
-               while IFS="|" read _x _ip _x _iface _x; do
+               while IFS="|" read -r _ _ip _ _iface _; do
                        [ -n "$_iface" ] || continue
-                       while IFS="/$IFS" read _i _maskbits _x; do
+                       while IFS="/$IFS" read -r _i _maskbits _; do
                                if [ "$_ip" = "$_i" ]; then
-                                       echo $_iface $_ip $_maskbits
+                                       echo "$_iface $_ip $_maskbits"
                                        break
                                fi
                        done <"${CTDB_BASE}/public_addresses"
@@ -378,7 +382,7 @@ check_routes()
                                cat >/dev/null
                        }
                fi | {
-               while read _dev _ip _bits; do
+               while read -r _dev _ip _bits; do
                        _net=$(ipv4_host_addr_to_net "$_ip" "$_bits")
                        _gw="${_net%.*}.254" # a dumb, calculated default
 
@@ -517,15 +521,19 @@ define_test()
 
 simple_test()
 {
-       [ -n "$event" ] || die 'simple_test: $event not set'
+       [ -n "$event" ] || die 'simple_test: event not set'
 
-       args="$@"
+       args="$*"
 
+       # shellcheck disable=SC2317
+       # used in unit_test(), etc.
        test_header()
        {
                echo "Running script \"$script $event${args:+ }$args\""
        }
 
+       # shellcheck disable=SC2317
+       # used in unit_test(), etc.
        extra_header()
        {
                cat <<EOF
@@ -547,7 +555,7 @@ EOF
 simple_test_event()
 {
        # If something has previously failed then don't continue.
-       : ${_passed:=true}
+       : "${_passed:=true}"
        $_passed || return 1
 
        event="$1"
index e7541b571d97e4a98c00f5efd28263e94ea1eae2..e966cb4bf58ee10cf1898ccf44d020364a19c576 100644 (file)
@@ -9,9 +9,9 @@ ctdb_catdb_format_pairs()
 {
        _count=0
 
-       while read _k _v; do
-               _kn=$(echo -n "$_k" | wc -c)
-               _vn=$(echo -n "$_v" | wc -c)
+       while read -r _k _v; do
+               _kn=$(printf '%s' "$_k" | wc -c)
+               _vn=$(printf '%s' "$_v" | wc -c)
                cat <<EOF
 key(${_kn}) = "${_k}"
 dmaster: 0
@@ -19,7 +19,7 @@ rsn: 1
 data(${_vn}) = "${_v}"
 
 EOF
-               _count=$(($_count + 1))
+               _count=$((_count + 1))
        done
 
        echo "Dumped ${_count} records"
@@ -28,7 +28,7 @@ EOF
 check_ctdb_tdb_statd_state()
 {
        ctdb_get_my_public_addresses |
-               while read _x _sip _x; do
+               while read -r _ _sip _; do
                        for _cip; do
                                cat <<EOF
 statd-state@${_sip}@${_cip} $(date)
@@ -44,12 +44,12 @@ EOF
 check_statd_callout_smnotify()
 {
        _state_even=$(( $(date '+%s') / 2 * 2))
-       _state_odd=$(($_state_even + 1))
+       _state_odd=$((_state_even + 1))
 
        nfs_load_config
 
        ctdb_get_my_public_addresses |
-               while read _x _sip _x; do
+               while read -r _ _sip _; do
                        for _cip; do
                                cat <<EOF
 SM_NOTIFY: ${_sip} -> ${_cip}, MON_NAME=${_sip}, STATE=${_state_even}