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

Lars Kruse lists at sumpfralle.de
Sun Oct 2 05:07:12 PDT 2016


Hi Jan-Tarek,


Am Sat, 1 Oct 2016 21:37:14 +0200
schrieb Jan-Tarek Butt <tarek at ring0.de>:

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

You are right - I got lost in the detail and forgot the goal.
Maybe this is a hint for creating a function like "get_real_path" in order to
increase readability? (outside of the scope of this patch)

Combining this with Oswald's point regarding "exit in the subshell", I would
propose the following:

-	bindirs="$bindirs $(cd "$bindir"; pwd)/"
+	bindirs="$bindirs $(cd "$bindir" && printf "%s/" "$(pwd)")"

This would have the following effects for inaccessible directories:
* the error message of "cd" still goes to stderr
* "bindirs" is extended by one whitespace (irrelevant)
* the script continues

Previously the old code had almost the same effect. The only difference was that
the current directory (pwd) was added instead of any inaccessible directory.
Thus the change would probably be in line with the original intention and
improve this corner case.

Additionally forcing the script to exit on every inaccessible bindir/libdir/...
could potentially be the right thing. But this would be a change of behaviour
and I am not sure if it was intended.

Cheers,
Lars
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: Digitale Signatur von OpenPGP
URL: <http://lists.infradead.org/pipermail/lede-dev/attachments/20161002/bdad20e0/attachment.sig>


More information about the Lede-dev mailing list