[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