[LEDE-DEV] [patch master 05/15] Use cd ... || exit in case cd fails

[Jan-Tarek Butt]

On 10/01/16 00:05, Lars Kruse wrote:
> Hi,
> 
> 
> Am Fri, 30 Sep 2016 22:02:19 +0200
> schrieb Jan-Tarek Butt <tarek at ring0.de>:
> 
> 
>> diff --git a/scripts/clean-package.sh b/scripts/clean-package.sh
>> index 5cae341..9efcac5 100755
>> --- a/scripts/clean-package.sh
>> +++ b/scripts/clean-package.sh
>> @@ -8,14 +8,14 @@
>>  	exit 1
>>  }
>>  cat "$1" | (
>> -	cd "$2"
>> +	cd "$2" || exit
>>  	while read entry; do
>>  		[ -n "$entry" ] || break
>>  		[ -f "$entry" ] && rm -f "$entry"
>>  	done
>>  )
>>  cat "$1" | (
>> -	cd "$2"
>> +	cd "$2" || exit
>>  	while read entry; do
>>  		[ -n "$entry" ] || break
>>  		[ -d "$entry" ] && rmdir "$entry" > /dev/null 2>&1
> 
> personally I would prefer "set -e" at the top of this script instead.
> Since this script is removing files, I would take the safe path of exiting on
> any error ("set -e") instead of manually checking for errors and exiting for
> certain commands.

OK,

>> diff --git a/scripts/deptest.sh b/scripts/deptest.sh
>> index d7be99d..cce7caa 100755
>> --- a/scripts/deptest.sh
>> +++ b/scripts/deptest.sh
>> @@ -115,9 +115,9 @@ test_package() # $1=pkgname
>>  	local logfile="$(basename "$pkg").log"
>>  	deptest_make "package/$pkg/compile" "$logfile"
>>  	if [ $? -eq 0 ]; then
>> -		( cd "$STAMP_DIR_SUCCESS"; ln -s "../$LOG_DIR_NAME/$logfile"
>> "./$pkg" )
>> +		( cd "$STAMP_DIR_SUCCESS" || exit; ln -s
>> "../$LOG_DIR_NAME/$logfile" "./$pkg" ) else
>> -		( cd "$STAMP_DIR_FAILED"; ln -s "../$LOG_DIR_NAME/$logfile"
>> "./$pkg" )
>> +		( cd "$STAMP_DIR_FAILED" || exit; ln -s
>> "../$LOG_DIR_NAME/$logfile" "./$pkg" ) echo "Building package $pkg FAILED"
>>  	fi
>>  }
> 
> In this specific shell script there is a function "die" - probably you should
> use it instead of "exit".

Thanks I'll change it.

>> diff --git a/scripts/ext-toolchain.sh b/scripts/ext-toolchain.sh
>> index 8d8379e..9cd6e45 100755
>> --- a/scripts/ext-toolchain.sh
>> +++ b/scripts/ext-toolchain.sh
>> @@ -151,7 +151,7 @@ find_libs() {
>>  				sed -ne 's#:# #g; s#^LIBRARY_PATH=##p'
>>  		); do
>>  			if [ -d "$libdir" ]; then
>> -				libdirs="$libdirs $(cd "$libdir"; pwd)/"
>> +				libdirs="$libdirs $(cd "$libdir" || exit;
>> pwd)/" fi
> 
> I am not sure, if this script is supposed to break on every missing libdir (I
> do not know the context). Thus maybe " && " would be better instead ";"?
> This would prevent unwanted directories (e.g. the current one) to be added to
> the list and it would also not add non-existing directories to the list, as
> well.
> Just the error messages of failing "cd" operations would be a remaining
> annoyance.
> Maybe someone with a better feeling for this scripts knows what the proper
> reaction on a missing directory should be?

I think you miss understoud the code part. The if statemend
checks if the dir exsist. Inside the next line the val de
trys to ender the dir only if the entering of the dir fails
the program will terminated. So I dont see problems on the patch?

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/9e19464a/attachment.sig>