[iproute PATCH v2 0/2] Covscan: Shell script fixes

This series collects patches from v1 which deal with programming
mistakes in shell scripts.

No changes to the actual patches, just splitting into smaller series.

Phil Sutter (2):
  examples: Some shell fixes to cbq.init
  ifcfg: Quote left-hand side of [ ] expression

 examples/cbq.init-v0.7.3 | 24 ++++++++++++------------
 ip/ifcfg                 |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

-- 
2.13.1

This addresses the following issues:

- $@ is an array, so don't use it in quoted strings - use $* instead.

- Add missing quotes to components of [ ] expressions. These are not
  strictly necessary since the output of 'wc -l' should be a single word
  only, but in case of errors, bash prints "integer expression expected"
  instead of "too many arguments".

- Use -print0/-0 when piping from find to xargs to allow for filenames
  which contain whitespace.

- Quote arguments to 'eval' to prevent word-splitting.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 examples/cbq.init-v0.7.3 | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/examples/cbq.init-v0.7.3 b/examples/cbq.init-v0.7.3
index 1bc0d446f8983..66448d88f0053 100644
--- a/examples/cbq.init-v0.7.3
+++ b/examples/cbq.init-v0.7.3

@@ -532,7 +532,7 @@ cbq_off () {
 
 ### Prefixed message
 cbq_message () {
-	echo -e "**CBQ: $@"
+	echo -e "**CBQ: $*"
 } # cbq_message
 
 ### Failure message

@@ -560,15 +560,15 @@ cbq_time2abs () {
 ### Display CBQ setup
 cbq_show () {
 	for dev in `cbq_device_list`; do
-		[ `tc qdisc show dev $dev| wc -l` -eq 0 ] && continue
+		[ "`tc qdisc show dev $dev| wc -l`" -eq 0 ] && continue
 		echo -e "### $dev: queueing disciplines\n"
 		tc $1 qdisc show dev $dev; echo
 
-		[ `tc class show dev $dev| wc -l` -eq 0 ] && continue
+		[ "`tc class show dev $dev| wc -l`" -eq 0 ] && continue
 		echo -e "### $dev: traffic classes\n"
 		tc $1 class show dev $dev; echo
 
-		[ `tc filter show dev $dev| wc -l` -eq 0 ] && continue
+		[ "`tc filter show dev $dev| wc -l`" -eq 0 ] && continue
 		echo -e "### $dev: filtering rules\n"
 		tc $1 filter show dev $dev; echo
 	done

@@ -585,7 +585,7 @@ cbq_init () {
 
 	### Gather all DEVICE fields from $1/cbq-*
 	DEVFIELDS=`find $1 -maxdepth 1 \( -type f -or -type l \) -name 'cbq-*' \
-		  -not -name '*~' | xargs sed -n 's/#.*//; \
+		  -not -name '*~' -print0 | xargs -0 sed -n 's/#.*//; \
 		  s/[[:space:]]//g; /^DEVICE=[^,]*,[^,]*\(,[^,]*\)\?/ \
 		  { s/.*=//; p; }'| sort -u`
 	[ -z "$DEVFIELDS" ] &&

@@ -593,7 +593,7 @@ cbq_init () {
 
 	### Check for different DEVICE fields for the same device
 	DEVICES=`echo "$DEVFIELDS"| sed 's/,.*//'| sort -u`
-	[ `echo "$DEVICES"| wc -l` -ne `echo "$DEVFIELDS"| wc -l` ] &&
+	[ "`echo "$DEVICES"| wc -l`" -ne "`echo "$DEVFIELDS"| wc -l`" ] &&
 		cbq_failure "different DEVICE fields for single device!\n$DEVFIELDS"
 } # cbq_init
 

@@ -618,7 +618,7 @@ cbq_load_class () {
 	PRIO_MARK=$PRIO_MARK_DEFAULT
 	PRIO_REALM=$PRIO_REALM_DEFAULT
 
-	eval `echo "$CFILE"| grep -E "^($CBQ_WORDS)="`
+	eval "`echo "$CFILE"| grep -E "^($CBQ_WORDS)="`"
 
 	### Require RATE/WEIGHT
 	[ -z "$RATE" -o -z "$WEIGHT" ] &&

@@ -661,7 +661,7 @@ if [ "$1" = "compile" ]; then
 
 	### echo-only version of "tc" command
 	tc () {
-		echo "$TC $@"
+		echo "$TC $*"
 	} # tc
 
 elif [ -n "$CBQ_DEBUG" ]; then

@@ -669,13 +669,13 @@ elif [ -n "$CBQ_DEBUG" ]; then
 
 	### Logging version of "ip" command
 	ip () {
-		echo -e "\n# ip $@" >> $CBQ_DEBUG
+		echo -e "\n# ip $*" >> $CBQ_DEBUG
 		$IP "$@" 2>&1 | tee -a $CBQ_DEBUG
 	} # ip
 
 	### Logging version of "tc" command
 	tc () {
-		echo -e "\n# tc $@" >> $CBQ_DEBUG
+		echo -e "\n# tc $*" >> $CBQ_DEBUG
 		$TC "$@" 2>&1 | tee -a $CBQ_DEBUG
 	} # tc
 else

@@ -711,8 +711,8 @@ if [ "$1" != "compile" -a "$2" != "nocache" -a -z "$CBQ_DEBUG" ]; then
 	### validate the cache
 	[ "$2" = "invalidate" -o ! -f $CBQ_CACHE ] && VALID=0
 	if [ $VALID -eq 1 ]; then
-		[ `find $CBQ_PATH -maxdepth 1 -newer $CBQ_CACHE| \
-		  wc -l` -gt 0 ] && VALID=0
+		[ "`find $CBQ_PATH -maxdepth 1 -newer $CBQ_CACHE| \
+		  wc -l`" -gt 0 ] && VALID=0
 	fi
 
 	### compile the config if the cache is invalid

-- 
2.13.1

This prevents word-splitting and therefore leads to more accurate error
message in case 'grep -c' prints something other than a number.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ifcfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/ifcfg b/ip/ifcfg
index 083d9df36742f..30a2dc49816cd 100644
--- a/ip/ifcfg
+++ b/ip/ifcfg

@@ -131,7 +131,7 @@ noarp=$?
 
 ip route add unreachable 224.0.0.0/24 >& /dev/null
 ip route add unreachable 255.255.255.255 >& /dev/null
-if [ `ip link ls $dev | grep -c MULTICAST` -ge 1 ]; then
+if [ "`ip link ls $dev | grep -c MULTICAST`" -ge 1 ]; then
   ip route add 224.0.0.0/4 dev $dev scope global >& /dev/null
 fi
 

-- 
2.13.1

From: Phil Sutter

You could drag all these scripts into the 1990's by using $(...)
instead of `...`.

	David

On Fri, Aug 18, 2017 at 09:32:52AM +0000, David Laight wrote:

From: Phil Sutter

quoted

Sent: 17 August 2017 18:10
This prevents word-splitting and therefore leads to more accurate error
message in case 'grep -c' prints something other than a number.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ifcfg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ip/ifcfg b/ip/ifcfg
index 083d9df36742f..30a2dc49816cd 100644
--- a/ip/ifcfg
+++ b/ip/ifcfg

@@ -131,7 +131,7 @@ noarp=$?

 ip route add unreachable 224.0.0.0/24 >& /dev/null
 ip route add unreachable 255.255.255.255 >& /dev/null
-if [ `ip link ls $dev | grep -c MULTICAST` -ge 1 ]; then
+if [ "`ip link ls $dev | grep -c MULTICAST`" -ge 1 ]; then

You could drag all these scripts into the 1990's by using $(...)
instead of `...`.

That's a different kettle of fish IMO, using $() doesn't change the
situation this addresses:

| $ [ $(echo foo bar) -eq 0 ] && echo ok
| bash: [: too many arguments
| $ [ "$(echo foo bar)" -eq 0 ] && echo ok
| bash: [: foo bar: integer expression expected

Cheers, Phil

On Thu, 17 Aug 2017 19:09:30 +0200
Phil Sutter [off-list ref] wrote:

This series collects patches from v1 which deal with programming
mistakes in shell scripts.

No changes to the actual patches, just splitting into smaller series.

Phil Sutter (2):
  examples: Some shell fixes to cbq.init
  ifcfg: Quote left-hand side of [ ] expression

 examples/cbq.init-v0.7.3 | 24 ++++++++++++------------
 ip/ifcfg                 |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

Applied.

From: Phil Sutter

Sent: 18 August 2017 12:24

...

quoted

quoted

-if [ `ip link ls $dev | grep -c MULTICAST` -ge 1 ]; then
+if [ "`ip link ls $dev | grep -c MULTICAST`" -ge 1 ]; then

You could drag all these scripts into the 1990's by using $(...)
instead of `...`.

That's a different kettle of fish IMO, using $() doesn't change the
situation this addresses:

| $ [ $(echo foo bar) -eq 0 ] && echo ok
| bash: [: too many arguments
| $ [ "$(echo foo bar)" -eq 0 ] && echo ok
| bash: [: foo bar: integer expression expected

I didn't say it would.
IFS=
set -o noglob
would though - not that I'd suggest it here.
protecting the 'eval' is somewhat harder.

	David