fai-cd: quotes around command for -x test

[Michael Prokop]

* Michael Tautschnig <mt at debian.org> [Wed Oct 13, 2010 at 10:23:00PM +0100]:

> > There should be quotes around the $(command) for the -x test shell
> > builtin: [ -x "$(command)" ]
> > This is owing to the following:

> > debian:~# which rsync
> > /usr/bin/rsync
> > debian:~# which rsync_x
> > debian:~# [ -x "$(which rsync)" ] && echo ok
> > ok
> > debian:~# [ -x "$(which rsync_x)" ] && echo ok
> > debian:~# [ -x $(which rsync) ] && echo ok
> > ok
> > debian:~# [ -x $(which rsync_x) ] && echo ok
> > ok
> > debian:~#

> Thanks a lot for debugging this; in fact I wonder whether you do some kind of
> (automated?) code inspection or what is it that makes you discover all these
> hidden issues!?

> Furthermore, thanks for providing the nice counter-example. I must, however,
> admit that I don't quite like the patch. Or, rather, I'd like to go a bit
> further. The quotes are fine and maybe even necessary to catch cases where the
> path to rsync contains whitespace. This, however, will likely cause a number of
> other errors and won't be seen on too many systems. To catch the more likely
> case of a missing rsync command I'd suggest the following:

> which rsync && [ -x $(which rsync) ] && echo ok

> (Well, maybe even a which rsync && echo ok would be just as fine.)

> Comments are welcome!

Thanks for bringing up this issue. Actually "&&" (and also "||") is
quite dangerous in shell scripting overall because it tends to hide
issues many people aren't aware of. Let me explain.

Compare this "&&" version:

,---- [ % cat demo1.sh ]
| set -e
|
| some_function() {
|   echo do some magic
|   [ -x "$(which rsync42)" ] && echo some-ok
| }
|
| another_function() {
|   echo do another magic
|   [ -x "$(which rsync42)" ] && echo another-ok
| }
|
| some_function ; another_function
`----

with this "if .. then " version:

,---- [ % cat demo2.sh ]
| set -e
|
| some_function() {
|   echo do some magic
|   if [ -x "$(which rsync42)" ] ; then echo some-ok ; fi
| }
|
| another_function() {
|   echo do another magic
|   if [ -x "$(which rsync42)" ] ; then echo another-ok ; fi
| }
|
| some_function ; another_function
`----

Now think a second about what output you'd expect to get.


Well:

% bash ./demo1.sh ; echo $?
do some magic
1

% bash ./demo2.sh ; echo $?
do some magic
do another magic
0


This example might be too obvious but it's a pretty serious problem
I commonly find in code. It's especially dangerous when code relies
on return codes of other functions and a "some_check && echo foo" is
present at the end of a function (which might just happen by
accident when rewriting code).

Actually I can even remember this problem in FAI's lib/subroutines
where my fix for a wrong return code issue at the end of a function
was:

-   [ "$task_error" -ne 0 ] && echo "Exit code task_$taskname: $task_error"
+        if [ "$task_error" -ne 0 ] ; then
+          echo "Exit code task_$taskname: $task_error"
+        fi

Through negation and proper use of "||" you can get around this
issue as well:

  [[ $a = $b ]] && … → [[ $a != $b ]] || …

- but I noticed that too many people aren't awware of that.

So please prefer "if..then" over "&&" unless you're 100% sure
nothing can go wrong. :)

regards,
-mika-
-- 
http://michael-prokop.at/  || http://adminzen.org/
http://grml-solutions.com/ || http://grml.org/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
Url : http://lists.uni-koeln.de/pipermail/linux-fai-devel/attachments/20101014/ea870139/attachment.bin