Package: ifupdown-extra
Version: 0.21
Severity: wishlist
Tag: patch

Dear Maintainer,

   * What led up to the situation?
     + shell script sanity

   * What exactly did you do (or not do) that was effective (or ineffective)?
     + tough :)  did not install the package

   * What was the outcome of this action?
     + a patch; attached

   * What outcome did you expect instead?
     + sanity

-- System Information:
Debian Release: wheezy/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing')
Architecture: i386 (i686)

Kernel: Linux 3.0.0-1-686-pae (SMP w/2 CPU cores)
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
Shell: /bin/sh linked to /bin/bash


Cheers,

-- 
Cristian
commit d059bd8112d8e9176fd8e5339f268599c24f09bb
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 18:40:02 2011 +0200

    Cleaned up whitespace damage.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 976760a..c265aec 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -2,7 +2,7 @@
 # Check the link status of an ethernet interface
 # This script should be installed in /etc/network/if-pre-up.d/
 #
-# You can use this script to solve bug #120382 
+# You can use this script to solve bug #120382
 # ('ifup should (optionally) check for link before configuring the interface.')
 # if you configure ABORT_NO_LINK to 'yes' in /etc/default/network-test
 # since this will make the script abort if the interface does not have
@@ -25,7 +25,7 @@
 #   You should have received a copy of the GNU General Public License
 #   along with this program; if not, write to the Free Software
 #   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
-#  
+#
 # You can also find a copy of the GNU General Public License at
 # http://www.gnu.org/licenses/licenses.html#TOCLGPL
 #
@@ -51,35 +51,37 @@ fi
 LC_ALL=C
 export LC_ALL
 
-check_status_miitool () {
+check_status_miitool() {
 	local status=0
+
 	if $MIITOOL $IFACE 2>&1| grep -q "no link"; then
 		status=1
 	fi
 	return $status
 }
 
-check_status_ethtool () {
+check_status_ethtool() {
 	local status=0
 	local LINK="`$ETHTOOL $IFACE 2>&1| grep \"Link detected\"`"
+
 	# If ethtool fails to print out the link line we break off
 	# notice that ethtool cannot get the link status out of all
 	# possible network interfaces
 	[ -z "$LINK" ] && return
-	if ! echo $LINK | grep -q "Link detected: yes" ; then
+	if ! echo $LINK | grep -q "Link detected: yes"; then
 		status=1
 	fi
 	return $status
 }
 
-check_status_iplink () {
+check_status_iplink() {
 	local status=0
 	local info="`/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" `"
+
 	[ ! -x /sbin/ip ] && return 0
 	if echo $info | grep -q "NO-CARRIER" ||
 	   echo $info | grep -q "state DOWN" ||
-	   echo $info | grep -q "state LOWERLAYERDOWN"
-	then
+	   echo $info | grep -q "state LOWERLAYERDOWN"; then
 		status=1
 	fi
 	return $status
@@ -87,15 +89,16 @@ check_status_iplink () {
 
 check_status() {
 	local status=0
-        local myid="`id -u`"
+	local myid="`id -u`"
+
 	ifconfig $IFACE 2>/dev/null 1>&2
-	if [ $? -ne 0 ] ; then
+	if [ $? -ne 0 ]; then
 		$OUTPUT "ERROR: Interface $IFACE does not seem to be present in the system"
 		return
 	fi
 	# Use ethtool if installed (preferable to mii-tool)
 	# If none are installed (or not running as root) we will test using 'ip link show'
-	if [ -x "$ETHTOOL" ] && [ $myid = 0 ] ; then
+	if [ -x "$ETHTOOL" ] && [ $myid = 0 ]; then
 		check_status_ethtool
 		status=$?
 	elif [ -x "$MIITOOL" ] && [ $myid = 0 ]; then
@@ -105,7 +108,7 @@ check_status() {
 		check_status_iplink
 		status=$?
 	fi
-	if [ $status -ne 0 ] ; then
+	if [ $status -ne 0 ]; then
 		$OUTPUT "WARNING: Initialising interface $IFACE which does not have link"
 	fi
 	return $status
@@ -113,12 +116,12 @@ check_status() {
 
 check_bond_status() {
 	local status=1
-        [ ! -e /sys/class/net/$IFACE/bonding/slaves ] && return 0
+	[ ! -e /sys/class/net/$IFACE/bonding/slaves ] && return 0
 	for slave_iface in `cat /sys/class/net/$IFACE/bonding/slaves`; do
 		# Use "true" to silence slaves.
 		OUTPUT="true" IFACE="$slave_iface" check_status
 		status=$?
-		if [ $status -eq 0 ] ; then
+		if [ $status -eq 0 ]; then
 			# One functional slave will suffice
 			return 0
 		fi
@@ -127,25 +130,26 @@ check_bond_status() {
 	return $status
 }
 
-if [ -z "$IFACE" ] ; then
+if [ -z "$IFACE" ]; then
     echo "ERROR: Variable IFACE not set in environment" >&2
     exit 1
 fi
 
 # Check our IFACE name, if it does not start with eth, bail out
-case "$IFACE" in 
-	eth*) 
+case "$IFACE" in
+	eth*)
 		check_status
-		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = "yes" ] ; then
+		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = "yes" ]; then
 			exit 1
 		fi
 		;;
 	bond*)
 		check_bond_status
-		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = "yes" ] ; then
+		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = "yes" ]; then
 			exit 1
 		fi
 		;;
-	*) ;;
+	*)
+		;;
 esac
 exit 0
commit bb91aab01d84b0d88e3833ec6cd7e5c8f340503e
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 18:41:57 2011 +0200

    Separated 'local' declaration(s) with a newline...
    
    ...from the function body.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index c265aec..c081937 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -116,6 +116,7 @@ check_status() {
 
 check_bond_status() {
 	local status=1
+
 	[ ! -e /sys/class/net/$IFACE/bonding/slaves ] && return 0
 	for slave_iface in `cat /sys/class/net/$IFACE/bonding/slaves`; do
 		# Use "true" to silence slaves.
@@ -149,7 +150,5 @@ case "$IFACE" in
 			exit 1
 		fi
 		;;
-	*)
-		;;
 esac
 exit 0
commit 992ce8715e63f8addb562c84b0171973d800be82
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 18:46:36 2011 +0200

    Removed totally useless quotes.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index c081937..a6061e4 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -35,16 +35,16 @@
 
 # Defaults
 ETHTOOL=/sbin/ethtool
-[ ! -x "$ETHTOOL" ] && [ -x "/usr/sbin/ethtool" ] && ETHTOOL=/usr/sbin/ethtool
+[ ! -x $ETHTOOL ] && [ -x /usr/sbin/ethtool ] && ETHTOOL=/usr/sbin/ethtool
 MIITOOL=/sbin/mii-tool
 DO_SYSLOG=${DO_SYSLOG:-yes}
 ABORT_NO_LINK=i${ABORT_NO_LINK:-no}
 VERBOSITY=${VERBOSITY:-0}
 
-if [ "$DO_SYSLOG" = "yes" ] ; then
+if [ "$DO_SYSLOG" = yes ] ; then
 	OUTPUT="logger -i -p daemon.err -s"
 else
-	OUTPUT="echo"
+	OUTPUT=echo
 fi
 
 # Set our locale environment, just in case any of the tools get translated
@@ -79,7 +79,7 @@ check_status_iplink() {
 	local info="`/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" `"
 
 	[ ! -x /sbin/ip ] && return 0
-	if echo $info | grep -q "NO-CARRIER" ||
+	if echo $info | grep -q NO-CARRIER ||
 	   echo $info | grep -q "state DOWN" ||
 	   echo $info | grep -q "state LOWERLAYERDOWN"; then
 		status=1
@@ -98,10 +98,10 @@ check_status() {
 	fi
 	# Use ethtool if installed (preferable to mii-tool)
 	# If none are installed (or not running as root) we will test using 'ip link show'
-	if [ -x "$ETHTOOL" ] && [ $myid = 0 ]; then
+	if [ -x $ETHTOOL ] && [ $myid = 0 ]; then
 		check_status_ethtool
 		status=$?
-	elif [ -x "$MIITOOL" ] && [ $myid = 0 ]; then
+	elif [ -x $MIITOOL ] && [ $myid = 0 ]; then
 		check_status_miitool
 		status=$?
 	else
@@ -120,7 +120,7 @@ check_bond_status() {
 	[ ! -e /sys/class/net/$IFACE/bonding/slaves ] && return 0
 	for slave_iface in `cat /sys/class/net/$IFACE/bonding/slaves`; do
 		# Use "true" to silence slaves.
-		OUTPUT="true" IFACE="$slave_iface" check_status
+		OUTPUT=true IFACE=$slave_iface check_status
 		status=$?
 		if [ $status -eq 0 ]; then
 			# One functional slave will suffice
@@ -137,16 +137,16 @@ if [ -z "$IFACE" ]; then
 fi
 
 # Check our IFACE name, if it does not start with eth, bail out
-case "$IFACE" in
+case $IFACE in
 	eth*)
 		check_status
-		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = "yes" ]; then
+		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = yes ]; then
 			exit 1
 		fi
 		;;
 	bond*)
 		check_bond_status
-		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = "yes" ]; then
+		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = yes ]; then
 			exit 1
 		fi
 		;;
commit 52740ea4ae16ccc99df7720dadbe5d53fb2b656b
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 18:50:18 2011 +0200

    Test arith variable values using arith operators.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index a6061e4..f71a649 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -98,10 +98,10 @@ check_status() {
 	fi
 	# Use ethtool if installed (preferable to mii-tool)
 	# If none are installed (or not running as root) we will test using 'ip link show'
-	if [ -x $ETHTOOL ] && [ $myid = 0 ]; then
+	if [ -x $ETHTOOL ] && [ $myid -eq 0 ]; then
 		check_status_ethtool
 		status=$?
-	elif [ -x $MIITOOL ] && [ $myid = 0 ]; then
+	elif [ -x $MIITOOL ] && [ $myid -eq 0 ]; then
 		check_status_miitool
 		status=$?
 	else
commit 4318f3224198da80035d811387d38a936628403b
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 19:00:26 2011 +0200

    Avoid resource wasteful checks on $?.
    
    Also, use safer inverted logic in tests.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index f71a649..e95340f 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -91,22 +91,18 @@ check_status() {
 	local status=0
 	local myid="`id -u`"
 
-	ifconfig $IFACE 2>/dev/null 1>&2
-	if [ $? -ne 0 ]; then
+	ifconfig $IFACE 2>/dev/null 1>&2 || {
 		$OUTPUT "ERROR: Interface $IFACE does not seem to be present in the system"
 		return
-	fi
+	}
 	# Use ethtool if installed (preferable to mii-tool)
 	# If none are installed (or not running as root) we will test using 'ip link show'
 	if [ -x $ETHTOOL ] && [ $myid -eq 0 ]; then
-		check_status_ethtool
-		status=$?
+		check_status_ethtool || status=$?
 	elif [ -x $MIITOOL ] && [ $myid -eq 0 ]; then
-		check_status_miitool
-		status=$?
+		check_status_miitool || status=$?
 	else
-		check_status_iplink
-		status=$?
+		check_status_iplink || status=$?
 	fi
 	if [ $status -ne 0 ]; then
 		$OUTPUT "WARNING: Initialising interface $IFACE which does not have link"
@@ -120,8 +116,7 @@ check_bond_status() {
 	[ ! -e /sys/class/net/$IFACE/bonding/slaves ] && return 0
 	for slave_iface in `cat /sys/class/net/$IFACE/bonding/slaves`; do
 		# Use "true" to silence slaves.
-		OUTPUT=true IFACE=$slave_iface check_status
-		status=$?
+		OUTPUT=true IFACE=$slave_iface check_status || status=$?
 		if [ $status -eq 0 ]; then
 			# One functional slave will suffice
 			return 0
@@ -139,16 +134,10 @@ fi
 # Check our IFACE name, if it does not start with eth, bail out
 case $IFACE in
 	eth*)
-		check_status
-		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = yes ]; then
-			exit 1
-		fi
+		check_status || [ "$ABORT_NO_LINK" != yes ] || exit 1
 		;;
 	bond*)
-		check_bond_status
-		if [ $? -ne 0 ] && [ "$ABORT_NO_LINK" = yes ]; then
-			exit 1
-		fi
+		check_bond_status || [ "$ABORT_NO_LINK" != yes ] || exit 1
 		;;
 esac
 exit 0
commit 9050d7429ab9b435119131703acf9d510fe83809
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 19:02:58 2011 +0200

    Removed useless 'exit 1'.
    
    If exit status -ne 0 had occured, that point will never be reached
    anyway (provided proper error handling).
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index e95340f..de8c2a2 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -140,4 +140,3 @@ case $IFACE in
 		check_bond_status || [ "$ABORT_NO_LINK" != yes ] || exit 1
 		;;
 esac
-exit 0
commit f19336f49ef9d41c0748ab7121c67265b24a30d5
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 19:13:00 2011 +0200

    Use safer inverted logic, simplify and speed up somewhat.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index de8c2a2..28e128b 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -54,9 +54,7 @@ export LC_ALL
 check_status_miitool() {
 	local status=0
 
-	if $MIITOOL $IFACE 2>&1| grep -q "no link"; then
-		status=1
-	fi
+	! $MIITOOL $IFACE 2>&1| grep -q "no link" || status=1
 	return $status
 }
 
@@ -67,10 +65,8 @@ check_status_ethtool() {
 	# If ethtool fails to print out the link line we break off
 	# notice that ethtool cannot get the link status out of all
 	# possible network interfaces
-	[ -z "$LINK" ] && return
-	if ! echo $LINK | grep -q "Link detected: yes"; then
-		status=1
-	fi
+	[ "$LINK" ] || return
+	echo $LINK | grep -q "Link detected: yes" || status=1
 	return $status
 }
 
@@ -79,11 +75,9 @@ check_status_iplink() {
 	local info="`/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" `"
 
 	[ ! -x /sbin/ip ] && return 0
-	if echo $info | grep -q NO-CARRIER ||
-	   echo $info | grep -q "state DOWN" ||
-	   echo $info | grep -q "state LOWERLAYERDOWN"; then
-		status=1
-	fi
+	! echo $info | grep -q NO-CARRIER &&
+	! echo $info | grep -q "state DOWN" &&
+	! echo $info | grep -q "state LOWERLAYERDOWN" || status=1
 	return $status
 }
 
@@ -104,16 +98,15 @@ check_status() {
 	else
 		check_status_iplink || status=$?
 	fi
-	if [ $status -ne 0 ]; then
+	[ $status -eq 0 ] ||
 		$OUTPUT "WARNING: Initialising interface $IFACE which does not have link"
-	fi
 	return $status
 }
 
 check_bond_status() {
 	local status=1
 
-	[ ! -e /sys/class/net/$IFACE/bonding/slaves ] && return 0
+	[ -e /sys/class/net/$IFACE/bonding/slaves ] || return 0
 	for slave_iface in `cat /sys/class/net/$IFACE/bonding/slaves`; do
 		# Use "true" to silence slaves.
 		OUTPUT=true IFACE=$slave_iface check_status || status=$?
@@ -126,10 +119,10 @@ check_bond_status() {
 	return $status
 }
 
-if [ -z "$IFACE" ]; then
+[ "$IFACE" ] || {
     echo "ERROR: Variable IFACE not set in environment" >&2
     exit 1
-fi
+}
 
 # Check our IFACE name, if it does not start with eth, bail out
 case $IFACE in
commit b120d02e7f27726cf8bbc56b4e8288a55063ea97
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 19:26:06 2011 +0200

    Inverted logic is safer.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 28e128b..6364028 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -31,11 +31,11 @@
 #
 
 # Read system default file
-[ -r /etc/default/network-test ] && . /etc/default/network-test
+[ ! -r /etc/default/network-test ] || . /etc/default/network-test
 
 # Defaults
 ETHTOOL=/sbin/ethtool
-[ ! -x $ETHTOOL ] && [ -x /usr/sbin/ethtool ] && ETHTOOL=/usr/sbin/ethtool
+[ -x $ETHTOOL ] || [ ! -x /usr/sbin/ethtool ] || ETHTOOL=/usr/sbin/ethtool
 MIITOOL=/sbin/mii-tool
 DO_SYSLOG=${DO_SYSLOG:-yes}
 ABORT_NO_LINK=i${ABORT_NO_LINK:-no}
@@ -74,7 +74,7 @@ check_status_iplink() {
 	local status=0
 	local info="`/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" `"
 
-	[ ! -x /sbin/ip ] && return 0
+	[ -x /sbin/ip ] || return 0
 	! echo $info | grep -q NO-CARRIER &&
 	! echo $info | grep -q "state DOWN" &&
 	! echo $info | grep -q "state LOWERLAYERDOWN" || status=1
commit e43617b478e078af033f5e91f9e7aaf0b915bcdb
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 20:05:08 2011 +0200

    Unduplicate strings.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 6364028..ec252b7 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -31,11 +31,14 @@
 #
 
 # Read system default file
-[ ! -r /etc/default/network-test ] || . /etc/default/network-test
+rc=/etc/default/network-test
+[ ! -r $rc ] || . $rc
 
 # Defaults
 ETHTOOL=/sbin/ethtool
-[ -x $ETHTOOL ] || [ ! -x /usr/sbin/ethtool ] || ETHTOOL=/usr/sbin/ethtool
+
+alt_et=/usr/sbin/ethtool
+[ -x $ETHTOOL ] || [ ! -x $alt_et ] || ETHTOOL=$alt_et
 MIITOOL=/sbin/mii-tool
 DO_SYSLOG=${DO_SYSLOG:-yes}
 ABORT_NO_LINK=i${ABORT_NO_LINK:-no}
@@ -104,10 +107,12 @@ check_status() {
 }
 
 check_bond_status() {
-	local status=1
+	local status=1 slaves
+
+	slaves=/sys/class/net/$IFACE/bonding/slaves
 
-	[ -e /sys/class/net/$IFACE/bonding/slaves ] || return 0
-	for slave_iface in `cat /sys/class/net/$IFACE/bonding/slaves`; do
+	[ -e $slaves ] || return 0
+	for slave_iface in `cat $slaves`; do
 		# Use "true" to silence slaves.
 		OUTPUT=true IFACE=$slave_iface check_status || status=$?
 		if [ $status -eq 0 ]; then
commit 395e4b48c04a86e1ff52fc7ca012ad955954f1de
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 20:06:27 2011 +0200

    Inverted logic mantra continues :)
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index ec252b7..6f26ee9 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -115,10 +115,8 @@ check_bond_status() {
 	for slave_iface in `cat $slaves`; do
 		# Use "true" to silence slaves.
 		OUTPUT=true IFACE=$slave_iface check_status || status=$?
-		if [ $status -eq 0 ]; then
-			# One functional slave will suffice
-			return 0
-		fi
+		# One functional slave will suffice
+		[ $status -ne 0 ] || return 0
 	done
 	$OUTPUT "WARNING: Initialising bond $IFACE which does not have link on any slave"
 	return $status
commit ac81bcc65b407a33ad6f0f7a88e0f540be3208dd
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 20:09:18 2011 +0200

    Avoid a 'cat' fork.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 6f26ee9..502e3cd 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -112,12 +112,12 @@ check_bond_status() {
 	slaves=/sys/class/net/$IFACE/bonding/slaves
 
 	[ -e $slaves ] || return 0
-	for slave_iface in `cat $slaves`; do
+	while read slave_iface; do
 		# Use "true" to silence slaves.
 		OUTPUT=true IFACE=$slave_iface check_status || status=$?
 		# One functional slave will suffice
 		[ $status -ne 0 ] || return 0
-	done
+	done <$slaves
 	$OUTPUT "WARNING: Initialising bond $IFACE which does not have link on any slave"
 	return $status
 }
commit 45f151bb6241d27fcb2051ea34d26a772b911c15
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 20:12:18 2011 +0200

    Avoid backticks; modern shell scripts should avoid that.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 502e3cd..5a59ad2 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -63,7 +63,7 @@ check_status_miitool() {
 
 check_status_ethtool() {
 	local status=0
-	local LINK="`$ETHTOOL $IFACE 2>&1| grep \"Link detected\"`"
+	local LINK=$($ETHTOOL $IFACE 2>&1| grep \"Link detected\")
 
 	# If ethtool fails to print out the link line we break off
 	# notice that ethtool cannot get the link status out of all
@@ -75,7 +75,7 @@ check_status_ethtool() {
 
 check_status_iplink() {
 	local status=0
-	local info="`/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" `"
+	local info=$(/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" )
 
 	[ -x /sbin/ip ] || return 0
 	! echo $info | grep -q NO-CARRIER &&
@@ -86,7 +86,7 @@ check_status_iplink() {
 
 check_status() {
 	local status=0
-	local myid="`id -u`"
+	local myid=$(id -u)
 
 	ifconfig $IFACE 2>/dev/null 1>&2 || {
 		$OUTPUT "ERROR: Interface $IFACE does not seem to be present in the system"
commit 36135010e2e7002675bdffb67f299235696ba76f
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Wed Sep 14 20:16:09 2011 +0200

    Word splitting in a local declaration is buggy...
    
    ...in some shells (dash for example); avoid.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 5a59ad2..8d814b6 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -62,9 +62,9 @@ check_status_miitool() {
 }
 
 check_status_ethtool() {
-	local status=0
-	local LINK=$($ETHTOOL $IFACE 2>&1| grep \"Link detected\")
+	local status=0 LINK
 
+	LINK=$($ETHTOOL $IFACE 2>&1| grep \"Link detected\")
 	# If ethtool fails to print out the link line we break off
 	# notice that ethtool cannot get the link status out of all
 	# possible network interfaces
@@ -74,9 +74,9 @@ check_status_ethtool() {
 }
 
 check_status_iplink() {
-	local status=0
-	local info=$(/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" )
+	local status=0 info
 
+	info=$(/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" )
 	[ -x /sbin/ip ] || return 0
 	! echo $info | grep -q NO-CARRIER &&
 	! echo $info | grep -q "state DOWN" &&
@@ -85,8 +85,7 @@ check_status_iplink() {
 }
 
 check_status() {
-	local status=0
-	local myid=$(id -u)
+	local status=0 myid=$(id -u)
 
 	ifconfig $IFACE 2>/dev/null 1>&2 || {
 		$OUTPUT "ERROR: Interface $IFACE does not seem to be present in the system"
commit 9347216b3d54a771e3e46a9db6906a919cde7e53
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 16:59:31 2011 +0200

    Better error handling in 'check_status_ethtool'.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 8d814b6..973068a 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -64,11 +64,11 @@ check_status_miitool() {
 check_status_ethtool() {
 	local status=0 LINK
 
-	LINK=$($ETHTOOL $IFACE 2>&1| grep \"Link detected\")
+	LINK=$($ETHTOOL $IFACE 2>&1 | grep \"Link detected\" || :)
 	# If ethtool fails to print out the link line we break off
 	# notice that ethtool cannot get the link status out of all
 	# possible network interfaces
-	[ "$LINK" ] || return
+	[ "$LINK" ] || return 1
 	echo $LINK | grep -q "Link detected: yes" || status=1
 	return $status
 }
commit cfb589d50b184582087c00d45d54c77de69af7f3
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 17:04:25 2011 +0200

    Better error handling in 'check_status_iplink'...
    
    ...and a FIXME note.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 973068a..9d588d5 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -74,10 +74,11 @@ check_status_ethtool() {
 }
 
 check_status_iplink() {
-	local status=0 info
+	local ip_app=/sbin/ip status=0 info
 
-	info=$(/sbin/ip link show $IFACE 2>&1 | grep \"$IFACE:\" )
-	[ -x /sbin/ip ] || return 0
+	# FIXME: is this test really needed? The package depends on iproute!
+	[ -x $ip_app ] || return $status
+	info=$($ip_app link show $IFACE 2>&1 | grep \"$IFACE:\")
 	! echo $info | grep -q NO-CARRIER &&
 	! echo $info | grep -q "state DOWN" &&
 	! echo $info | grep -q "state LOWERLAYERDOWN" || status=1
commit c3bfa05e20ee12ceaad907cc9fc68a049f4080f9
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 17:08:43 2011 +0200

    One questionable return status.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 9d588d5..87c0445 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -90,6 +90,7 @@ check_status() {
 
 	ifconfig $IFACE 2>/dev/null 1>&2 || {
 		$OUTPUT "ERROR: Interface $IFACE does not seem to be present in the system"
+		# FIXME: would that be return status 0 or 1?
 		return
 	}
 	# Use ethtool if installed (preferable to mii-tool)
commit 684467403e5e6c4710bd84e0a23a033a00bcbab9
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 17:11:24 2011 +0200

    Chopped long lines.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index 87c0445..fc97f47 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -89,12 +89,14 @@ check_status() {
 	local status=0 myid=$(id -u)
 
 	ifconfig $IFACE 2>/dev/null 1>&2 || {
-		$OUTPUT "ERROR: Interface $IFACE does not seem to be present in the system"
+		$OUTPUT "ERROR: Interface $IFACE does not seem to be present" \
+			"in the system"
 		# FIXME: would that be return status 0 or 1?
 		return
 	}
 	# Use ethtool if installed (preferable to mii-tool)
-	# If none are installed (or not running as root) we will test using 'ip link show'
+	# If none are installed (or not running as root) we will test using
+	# 'ip link show'
 	if [ -x $ETHTOOL ] && [ $myid -eq 0 ]; then
 		check_status_ethtool || status=$?
 	elif [ -x $MIITOOL ] && [ $myid -eq 0 ]; then
@@ -103,7 +105,8 @@ check_status() {
 		check_status_iplink || status=$?
 	fi
 	[ $status -eq 0 ] ||
-		$OUTPUT "WARNING: Initialising interface $IFACE which does not have link"
+		$OUTPUT "WARNING: Initialising interface $IFACE which does" \
+			"not have link"
 	return $status
 }
 
@@ -119,7 +122,8 @@ check_bond_status() {
 		# One functional slave will suffice
 		[ $status -ne 0 ] || return 0
 	done <$slaves
-	$OUTPUT "WARNING: Initialising bond $IFACE which does not have link on any slave"
+	$OUTPUT "WARNING: Initialising bond $IFACE which does not have link" \
+		"on any slave"
 	return $status
 }
 
commit b01c8754729e3c6f75d04c53866850484b80eb72
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 17:15:55 2011 +0200

    Use guaranteed shell builtin ':' instead of 'true'.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index fc97f47..b653d6f 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -117,8 +117,8 @@ check_bond_status() {
 
 	[ -e $slaves ] || return 0
 	while read slave_iface; do
-		# Use "true" to silence slaves.
-		OUTPUT=true IFACE=$slave_iface check_status || status=$?
+		# Use ":" command to silence slaves.
+		OUTPUT=: IFACE=$slave_iface check_status || status=$?
 		# One functional slave will suffice
 		[ $status -ne 0 ] || return 0
 	done <$slaves
commit 04804e5f3df1c50905bd193142a07a8a56b84b00
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 17:18:50 2011 +0200

    Removed another useless SPACE.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index b653d6f..faedf12 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -44,7 +44,7 @@ DO_SYSLOG=${DO_SYSLOG:-yes}
 ABORT_NO_LINK=i${ABORT_NO_LINK:-no}
 VERBOSITY=${VERBOSITY:-0}
 
-if [ "$DO_SYSLOG" = yes ] ; then
+if [ "$DO_SYSLOG" = yes ]; then
 	OUTPUT="logger -i -p daemon.err -s"
 else
 	OUTPUT=echo
commit 6dcc5bcb0a441b5634493040d3ff7bd53cda59e3
Author: Cristian Ionescu-Idbohrn <c...@axis.com>
Date:   Sat Sep 17 17:20:39 2011 +0200

    Be consistent.
    
    Signed-off-by: Cristian Ionescu-Idbohrn <c...@axis.com>

diff --git a/if-up-scripts/check-network-cable b/if-up-scripts/check-network-cable
index faedf12..3f2276e 100755
--- a/if-up-scripts/check-network-cable
+++ b/if-up-scripts/check-network-cable
@@ -128,7 +128,7 @@ check_bond_status() {
 }
 
 [ "$IFACE" ] || {
-    echo "ERROR: Variable IFACE not set in environment" >&2
+    $OUTPUT "ERROR: Variable IFACE not set in environment"
     exit 1
 }
 

Reply via email to