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

[Jan-Tarek Butt]

Hi Lars,

On 09/30/16 23:50, Lars Kruse wrote:
> Hi Jan-Tarek,
> 
> first of all: I am very happy to see your patches!

Thanks :)

> 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!

I just do some small stuff :) Butt I plan to continiure work on it.

> 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.

Ah right, this kind are very special. Hm yes we can sed a comment on top.

> 
>>    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.

Thanks I'll fix it.

>> 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?

bacause there just nummeric.

>> -		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?

That command is to check the result of echo $spec

> I would assume that:
>  for pattern in $spec; do
> behaves exactly like:
>  for pattern in $(eval echo $spec); do

are you shure ?

> 
>> -				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.

Oh my mistake you are right.

>> 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.

Ah right I see the problem.
Thanks

Cheers
Tarek


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/lede-dev/attachments/20161001/14e7a2f6/attachment-0001.sig>