[LEDE-DEV] [patch master 01/15] Double quote to prevent globbing and word splitting

[Lars Kruse]

Hi Jan-Tarek,

first of all: I am very happy to see your patches!
Whenever I took a look at some shell scripts in openwrt, I felt the strong urge
to change many of the things you went out to fix. But until now I was always
stopped by the sheer amount of changes necessary. Thank you!


Am Fri, 30 Sep 2016 22:02:15 +0200
schrieb Jan-Tarek Butt <tarek at ring0.de>:

> [..]
> diff --git a/scripts/arm-magic.sh b/scripts/arm-magic.sh
> index 29ec88a..61ba098 100755
> --- a/scripts/arm-magic.sh
> +++ b/scripts/arm-magic.sh
> @@ -24,19 +24,19 @@
>  # list of supported boards, in "boardname machtypeid" format
>  for board in "avila 526" "gateway7001 731" "nslu2 597" "nas100d 865"
> "wg302v1 889" "wg302v2 890" "pronghorn 928" "pronghornmetro 1040" "compex
> 1273" "wrt300nv2 1077" "loft 849" "dsmg600 964" "fsg3 1091" "ap1000 1543"
> "tw2662 1658" "tw5334 1664" "ixdpg425 604" "cambria 1468" "sidewinder 1041"
> "ap42x 4418" do
> -  set -- $board
> -  hexid=$(printf %x\\n $2)
> +  set -- "$board"
> +  hexid=$(printf %x\\n "$2")

I think, exactly here the missing quotes were left out on purpose.
The "set -- ..." line was supposed to assign the string pairs above (e.g.
"avila 526" to $1 and $2. With your change both are squashed into $1 instead.
I do not see a good way use quoting here, thus I would recommend to add a
comment above explaining this (from my point of view: very exotic) "set"
statement.


>    if [ "$2" -lt "256" ]; then
>      # we have a low machtypeid, we just need a "mov" (e3a)
> -    printf "\xe3\xa0\x10\x$hexid" > $BIN_DIR/$IMG_PREFIX-$1-zImage
> +    printf "\xe3\xa0\x10\x$hexid" > "$BIN_DIR"/"$IMG_PREFIX"-"$1"-zImage

I would prefer the following:
 "$BIN_DIR/$IMG_PREFIX-$1-zImage"
over this:
 "$BIN_DIR"/"$IMG_PREFIX"-"$1"-zImage
But this is surely just a question of taste.


> diff --git a/scripts/combined-ext-image.sh b/scripts/combined-ext-image.sh
> index 374fe6e..b752aae 100755
> --- a/scripts/combined-ext-image.sh
> +++ b/scripts/combined-ext-image.sh
> @@ -38,7 +38,7 @@ IMG_OUT=$1; shift
>  FILE_NUM=$(($# / 2))
>  FILES=""
>  
> -printf "CE%02x%-32s%02x" $CE_VERSION "$IMG_TYPE" $FILE_NUM > $IMG_OUT
> +printf "CE%02x%-32s%02x" $CE_VERSION "$IMG_TYPE" $FILE_NUM > "$IMG_OUT"

Is there a reason for not adding quotes for CE_VERSION and FILE_NUM?



> -		for pattern in $(eval echo $spec); do
> -			find $libdirs -name "$pattern.so*" | sort -u
> +		for pattern in $(eval echo "$spec"); do
> +			find "$libdirs" -name "$pattern.so*" | sort -u

Just out of curiosity: do you know, what could be the purpose of the "eval"
construct above?
I would assume that:
 for pattern in $spec; do
behaves exactly like:
 for pattern in $(eval echo $spec); do



> -				exec "$CC" $CFLAGS -dumpmachine
> +				exec "$CC" "$CFLAGS" -dumpmachine

I think, that this change would squash all CFLAGS into a single parameter,
which should fail, I guess.
I cannot think of a way to add quotes here.


> diff --git a/scripts/strip-kmod.sh b/scripts/strip-kmod.sh
> index 313015b..ef35b82 100755
> --- a/scripts/strip-kmod.sh
> +++ b/scripts/strip-kmod.sh
> @@ -22,7 +22,7 @@ if [ -z "$KEEP_BUILD_ID" ]; then
>      ARGS="$ARGS -R .note.gnu.build-id"
>  fi
>  
> -${CROSS}objcopy \
> +"${CROSS}"objcopy \
>  	-R .comment \
>  	-R .pdr \
>  	-R .mdebug.abi32 \
> @@ -30,7 +30,7 @@ ${CROSS}objcopy \
>  	-R .reginfo \
>  	-R .MIPS.abiflags \
>  	-R .note.GNU-stack \
> -	$ARGS \
> +	"$ARGS" \

Based on the name "ARGS" I could imagine, that this variable contains more than
one argument. Thus the quotes would create a problem, I think.


The rest of these changes looked fine to me.

Cheers,
Lars