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

Lars Kruse lists at sumpfralle.de
Fri Sep 30 15:05:16 PDT 2016


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.


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


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

Cheers,
Lars



More information about the Lede-dev mailing list