From b208ae73d1f65bfd64c343f0396885fa8f5de404 Mon Sep 17 00:00:00 2001
From: Martin Schwenke <martin@meltin.net>
Date: Wed, 6 Jul 2016 17:41:55 +1000
Subject: [PATCH] ctdb-scripts: Avoid shellcheck warning SC2154 (unassigned
 variables)

SC2154: VAR is referenced but not assigned.

Change ctdb_setup_service_state_dir(), ctdb_get_pnn() and
ctdb_get_ip_address() to print the value so it can be assigned to a
variable.  The performance gain from avoiding the sub-shells when
calling these functions is close to zero.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
---
 ctdb/config/events.d/00.ctdb       |  2 --
 ctdb/config/events.d/05.system     |  2 +-
 ctdb/config/events.d/06.nfs        |  4 ++--
 ctdb/config/events.d/10.external   |  6 +++---
 ctdb/config/events.d/11.natgw      |  9 ++++-----
 ctdb/config/events.d/20.multipathd |  2 +-
 ctdb/config/events.d/49.winbind    |  2 --
 ctdb/config/events.d/50.samba      |  2 +-
 ctdb/config/events.d/60.nfs        |  4 ++--
 ctdb/config/events.d/70.iscsi      |  2 +-
 ctdb/config/events.d/91.lvs        |  7 +++----
 ctdb/config/functions              | 30 ++++++++++++++++--------------
 ctdb/config/statd-callout          |  8 +++-----
 13 files changed, 37 insertions(+), 43 deletions(-)

diff --git a/ctdb/config/events.d/00.ctdb b/ctdb/config/events.d/00.ctdb
index 175a44de6e3..777788b0b75 100755
--- a/ctdb/config/events.d/00.ctdb
+++ b/ctdb/config/events.d/00.ctdb
@@ -10,8 +10,6 @@
 
 loadconfig
 
-ctdb_setup_service_state_dir "ctdb"
-
 ############################################################
 
 select_tdb_checker ()
diff --git a/ctdb/config/events.d/05.system b/ctdb/config/events.d/05.system
index 43b472807ea..3931eaca345 100755
--- a/ctdb/config/events.d/05.system
+++ b/ctdb/config/events.d/05.system
@@ -8,7 +8,7 @@
 
 loadconfig
 
-ctdb_setup_service_state_dir "system-monitoring"
+service_state_dir=$(ctdb_setup_service_state_dir "system-monitoring") || exit $?
 
 validate_percentage ()
 {
diff --git a/ctdb/config/events.d/06.nfs b/ctdb/config/events.d/06.nfs
index 623911aefa7..0ecbb57f323 100755
--- a/ctdb/config/events.d/06.nfs
+++ b/ctdb/config/events.d/06.nfs
@@ -11,7 +11,7 @@
 service_name="nfs"
 
 loadconfig
-ctdb_setup_service_state_dir
+service_state_dir=$(ctdb_setup_service_state_dir) || exit $?
 
 ######################################################################
 
@@ -25,7 +25,7 @@ nfs_callout_pre ()
 
 ######################################################################
 
-nfs_callout_init
+nfs_callout_init "$service_state_dir"
 
 ctdb_start_stop_service
 
diff --git a/ctdb/config/events.d/10.external b/ctdb/config/events.d/10.external
index 7be301602f8..884357bc716 100644
--- a/ctdb/config/events.d/10.external
+++ b/ctdb/config/events.d/10.external
@@ -27,15 +27,15 @@ fi
 
 takeover_assigned_ips ()
 {
-    ctdb_get_pnn
+    _pnn=$(ctdb_get_pnn)
 
     $CTDB -X ip |
     awk -F'|' '{print $2}' |
     while read ip ; do
 	_ip_details=$(ip_maskbits_iface "$ip")
 	if [ -n "$_ip_details" ] ; then
-	    echo "Assigning $ip to this node ($pnn)"
-	    $CTDB moveip "$ip" "$pnn"
+	    echo "Assigning $ip to this node (${_pnn})"
+	    $CTDB moveip "$ip" "$_pnn"
 	fi
     done
 }
diff --git a/ctdb/config/events.d/11.natgw b/ctdb/config/events.d/11.natgw
index 61c93a3fa09..4dc023a1d08 100755
--- a/ctdb/config/events.d/11.natgw
+++ b/ctdb/config/events.d/11.natgw
@@ -19,7 +19,7 @@ loadconfig
 [ -n "$CTDB_NATGW_NODES" ] || exit 0
 export CTDB_NATGW_NODES
 
-ctdb_setup_service_state_dir
+service_state_dir=$(ctdb_setup_service_state_dir) || exit $?
 
 natgw_cfg_new="${service_state_dir}/cfg_new"
 natgw_cfg_old="${service_state_dir}/cfg_old"
@@ -27,9 +27,9 @@ natgw_master_old="${service_state_dir}/master_old"
 
 ctdb_natgw_slave_only ()
 {
-    ctdb_get_ip_address
+    _ip_address=$(ctdb_get_ip_address)
 
-    awk -v my_ip="$ip_address" \
+    awk -v my_ip="$_ip_address" \
 	'$1 == my_ip { if ($2 ~ "slave-only") { exit 0 } else { exit 1 } }' \
 	"$CTDB_NATGW_NODES"
 }
@@ -209,14 +209,13 @@ startup)
 updatenatgw|ipreallocated)
 	natgw_check_config
 
-	ctdb_get_pnn
-
 	natgw_ensure_master
 
 	natgw_config_has_changed || natgw_master_has_changed || exit 0
 
 	natgw_clear
 
+	pnn=$(ctdb_get_pnn)
 	if [ "$pnn" = "$natgwmaster" ]; then
 	    natgw_set_master
 	else
diff --git a/ctdb/config/events.d/20.multipathd b/ctdb/config/events.d/20.multipathd
index 2c153946f1d..f12ead6bd05 100755
--- a/ctdb/config/events.d/20.multipathd
+++ b/ctdb/config/events.d/20.multipathd
@@ -19,7 +19,7 @@ loadconfig
 
 [ -n "$CTDB_MONITOR_MPDEVICES" ] || exit 0
 
-ctdb_setup_service_state_dir
+service_state_dir=$(ctdb_setup_service_state_dir) || exit $?
 
 multipath_fail="${service_state_dir}/fail"
 
diff --git a/ctdb/config/events.d/49.winbind b/ctdb/config/events.d/49.winbind
index 8fcccfca724..d95f90f02ac 100755
--- a/ctdb/config/events.d/49.winbind
+++ b/ctdb/config/events.d/49.winbind
@@ -14,8 +14,6 @@ service_name="winbind"
 
 loadconfig
 
-ctdb_setup_service_state_dir
-
 service_start ()
 {
     service "$CTDB_SERVICE_WINBIND" stop >/dev/null 2>&1
diff --git a/ctdb/config/events.d/50.samba b/ctdb/config/events.d/50.samba
index 245566ef27a..b521d00858f 100755
--- a/ctdb/config/events.d/50.samba
+++ b/ctdb/config/events.d/50.samba
@@ -30,7 +30,7 @@ service_name="samba"
 
 loadconfig
 
-ctdb_setup_service_state_dir
+service_state_dir=$(ctdb_setup_service_state_dir) || exit $?
 
 service_start ()
 {
diff --git a/ctdb/config/events.d/60.nfs b/ctdb/config/events.d/60.nfs
index ff313032432..15948f58a90 100755
--- a/ctdb/config/events.d/60.nfs
+++ b/ctdb/config/events.d/60.nfs
@@ -11,7 +11,7 @@
 service_name="nfs"
 
 loadconfig
-ctdb_setup_service_state_dir
+service_state_dir=$(ctdb_setup_service_state_dir) || exit $?
 
 ######################################################################
 
@@ -248,7 +248,7 @@ nfs_update_lock_info ()
 
 ######################################################################
 
-nfs_callout_init
+nfs_callout_init "$service_state_dir"
 
 ctdb_start_stop_service
 
diff --git a/ctdb/config/events.d/70.iscsi b/ctdb/config/events.d/70.iscsi
index d99a060cda4..0937a12a8a9 100755
--- a/ctdb/config/events.d/70.iscsi
+++ b/ctdb/config/events.d/70.iscsi
@@ -51,7 +51,7 @@ EOF
 	# Stop iSCSI daemon
 	killall -9 tgtd >/dev/null 2>/dev/null
 
-	ctdb_get_pnn
+	pnn=$(ctdb_get_pnn)
 	[ -n "$pnn" ] || die "Failed to get node pnn"
 
 	# Start iSCSI daemon
diff --git a/ctdb/config/events.d/91.lvs b/ctdb/config/events.d/91.lvs
index c758b732685..c9722059dde 100755
--- a/ctdb/config/events.d/91.lvs
+++ b/ctdb/config/events.d/91.lvs
@@ -19,9 +19,8 @@ fi
 
 lvs_slave_only ()
 {
-	ctdb_get_ip_address
-
-	awk -v my_ip="$ip_address" \
+	_ip_address=$(ctdb_get_ip_address)
+	awk -v my_ip="$_ip_address" \
 	    '$1 == my_ip { if ($2 ~ "slave-only") { exit 0 } else { exit 1 } }' \
 	    "$CTDB_LVS_NODES"
 }
@@ -80,7 +79,7 @@ ipreallocated)
 	kill_tcp_connections_local_only \
 		"$CTDB_LVS_PUBLIC_IFACE" "$CTDB_LVS_PUBLIC_IP"
 
-	ctdb_get_pnn
+	pnn=$(ctdb_get_pnn)
 	lvsmaster=$("${CTDB_HELPER_BINDIR}/ctdb_lvs" master)
 	if [ "$pnn" != "$lvsmaster" ] ; then
 	    # This node is not the LVS master so change the IP address
diff --git a/ctdb/config/functions b/ctdb/config/functions
index bbf29e73d23..d3c4c59219c 100755
--- a/ctdb/config/functions
+++ b/ctdb/config/functions
@@ -245,7 +245,6 @@ nice_service()
 ######################################################
 # Cached retrieval of PNN from local node.  This never changes so why
 # open a client connection to the server each time this is needed?
-# This sets $pnn - this avoid an unnecessary subprocess.
 ctdb_get_pnn ()
 {
     _pnn_file="${CTDB_SCRIPT_VARDIR}/my-pnn"
@@ -253,11 +252,11 @@ ctdb_get_pnn ()
 	$CTDB pnn | sed -e 's@.*:@@' >"$_pnn_file"
     fi
 
-    read pnn <"$_pnn_file"
+    cat "$_pnn_file"
 }
 
 # Cached retrieval of private IP address from local node.  This never
-# changes.  Sets $ip_address to avoid an unnecessary subprocess.
+# changes.
 ctdb_get_ip_address ()
 {
     _ip_addr_file="${CTDB_SCRIPT_VARDIR}/my-ip-address"
@@ -268,7 +267,7 @@ ctdb_get_ip_address ()
 
     # ip_address is used by caller
     # shellcheck disable=SC2034
-    read ip_address <"$_ip_addr_file"
+    cat "$_ip_addr_file"
 }
 
 ######################################################
@@ -782,11 +781,13 @@ ctdb_counter_get () {
 
 ctdb_setup_service_state_dir ()
 {
-    service_state_dir="${CTDB_SCRIPT_VARDIR}/service_state/${1:-${service_name}}"
-    mkdir -p "$service_state_dir" || {
-	echo "Error creating state dir \"$service_state_dir\""
-	exit 1
-    }
+	_s="${1:-${service_name}}"
+
+	_service_state_dir="${CTDB_SCRIPT_VARDIR}/service_state/${_s}"
+	mkdir -p "$_service_state_dir" ||
+		die "Error creating state dir \"${_service_state_dir}\""
+
+	echo "$_service_state_dir"
 }
 
 ########################################################
@@ -1161,6 +1162,8 @@ fi
 
 nfs_callout_init ()
 {
+	_state_dir="$1"
+
 	if [ -z "$CTDB_NFS_CALLOUT" ] ; then
 		CTDB_NFS_CALLOUT="${CTDB_BASE}/nfs-linux-kernel-callout"
 	fi
@@ -1168,7 +1171,7 @@ nfs_callout_init ()
 	export CTDB_NFS_CALLOUT
 
 	# If the callout wants to use this then it must create it
-	export CTDB_NFS_CALLOUT_STATE_DIR="${service_state_dir}/callout-state"
+	export CTDB_NFS_CALLOUT_STATE_DIR="${_state_dir}/callout-state"
 
 	# Export, if set, for use by clustered NFS callouts
 	if [ -n "$CTDB_NFS_STATE_FS_TYPE" ] ; then
@@ -1178,7 +1181,7 @@ nfs_callout_init ()
 		export CTDB_NFS_STATE_MNT
 	fi
 
-	nfs_callout_cache="${service_state_dir}/nfs_callout_cache"
+	nfs_callout_cache="${_state_dir}/nfs_callout_cache"
 	nfs_callout_cache_callout="${nfs_callout_cache}/CTDB_NFS_CALLOUT"
 	nfs_callout_cache_ops="${nfs_callout_cache}/ops"
 }
@@ -1230,10 +1233,9 @@ update_tickles ()
 	tickledir="${CTDB_SCRIPT_VARDIR}/tickles"
 	mkdir -p "$tickledir"
 
-	ctdb_get_pnn
-
 	# What public IPs do I hold?
-	_ips=$($CTDB -X ip | awk -F'|' -v pnn="$pnn" '$3 == pnn {print $2}')
+	_pnn=$(ctdb_get_pnn)
+	_ips=$($CTDB -X ip | awk -F'|' -v pnn="$_pnn" '$3 == pnn {print $2}')
 
 	# IPs and port as ss filters
 	_ip_filter=""
diff --git a/ctdb/config/statd-callout b/ctdb/config/statd-callout
index a300255968d..e40a8aa62e6 100755
--- a/ctdb/config/statd-callout
+++ b/ctdb/config/statd-callout
@@ -30,11 +30,13 @@ loadconfig nfs
 nl="
 "
 
-ctdb_setup_service_state_dir "statd-callout"
+service_state_dir=$(ctdb_setup_service_state_dir "statd-callout") || exit $?
 
 cd "$service_state_dir" || \
     die "Failed to change directory to \"${service_state_dir}\""
 
+pnn=$(ctdb_get_pnn)
+
 case "$1" in
     # Keep a single file to keep track of the last "add-client" or
     # "del-client'.  These get pushed to ctdb.tdb during "update",
@@ -47,7 +49,6 @@ case "$1" in
 	# statd does not tell us to which IP the client connected so
 	# we must add it to all the IPs that we serve
 	cip="$2"
-	ctdb_get_pnn
 	date=$(date '+%s')
 	# x is intentionally ignored
 	# shellcheck disable=SC2034
@@ -64,7 +65,6 @@ case "$1" in
 	# statd does not tell us from which IP the client disconnected
 	# so we must add it to all the IPs that we serve
 	cip="$2"
-	ctdb_get_pnn
 	# x is intentionally ignored
 	# shellcheck disable=SC2034
 	$CTDB ip -X |
@@ -84,7 +84,6 @@ case "$1" in
 	fi
 	# Filter out lines for any IP addresses that are not currently
 	# hosted public IP addresses.
-	ctdb_get_pnn
 	ctdb_ips=$($CTDB ip | tail -n +2)
 	sed_expr=$(echo "$ctdb_ips" |
 	    awk -v pnn="$pnn" 'pnn == $2 { \
@@ -160,7 +159,6 @@ case "$1" in
 	# probability that the client will accept the statd notify packet and
 	# not just ignore it.
 	# For all IPs we serve, collect info and push to the config database
-	ctdb_get_pnn
 
 	# Construct a sed expression to take catdb output and produce pairs of:
 	#   server-IP client-IP