* Michael Tautschnig <m...@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/
signature.asc
Description: Digital signature