fai-cd: quotes around command for -x test

[michael log]

Helo Michael,

On Thu, Oct 14, 2010 at 12:23 AM, Michael Tautschnig <mt at debian.org> wrote:
>> There should be quotes around the $(command) for the -x test shell
>> builtin: [ -x "$(command)" ]
>> This is owing to the following:
>>
>
> [...]
>
> 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!?

I don't use any kind of automated code inspection (unfortunately; it
would be great if there was something like this, and perhaps there is
something, but I don't know it). My task now is to understand how it
is possible to do things that the FAI does. And all those discovered
issues are just from looking through the code and probably (I suspect)
from my previous working experience (sounds like self-advertisement
for a mailing list, but nevertheless...)

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

About the style of this correction. I agree that the solution is not
the perfect one, but. If you looked through the whole codebase you can
spot a lot of such constructions [ -x "$(command)" ]. And I tried to
be consistent with the methods which has been used in the similar
situations in this code. E.g.:

debian07:~/.temp/fai-sources# svn info
Path: .
URL: svn://svn.debian.org/svn/fai/trunk
Repository Root: svn://svn.debian.org/svn/fai
Repository UUID: ba5ec265-b0fb-0310-8e1a-cf9e4c2b1591
Revision: 6123
Node Kind: directory
Schedule: normal
Last Changed Author: lange
Last Changed Rev: 6118
Last Changed Date: 2010-10-06 10:45:09 -0400 (Wed, 06 Oct 2010)
debian07:~/.temp/fai-sources# grep -rn " \-x " ./  | grep which
./lib/task_sysinfo:14:[ -x "$(which dmidecode)" ] && dmidecode
./lib/task_sysinfo:15:[ -x "$(which lshw)" ] && lshw -short
./lib/task_sysinfo:17:[ -x "$(which discover)" ] && {
./lib/task_sysinfo:24:[ -x "$(which hwinfo)" ] && hwinfo --short
./lib/task_sysinfo:33:[ -x "$(which sfdisk)" ] && sfdisk -d
./lib/task_sysinfo:41:[ -x "$(which blkid)" ] && blkid
./bin/make-fai-nfsroot:525:[ ! -x "$(which debootstrap)" ] && die 1
"Can't find debootstrap command. Aborting."
./bin/fai-cd:334:[ -x "$(which rsync)" ] && rsync=1
./bin/fai-cd:376:    [ -x "$(which mkisofs)" ] || die 8 "mkisofs not
found. Please install package."
./examples/simple/class/10-base-classes:5:[ -x "`which dpkg`" ] &&
dpkg --print-architecture | tr a-z A-Z
debian07:~/.temp/fai-sources#

-- 
Regards,
Michael