On Tuesday 27 May 2008, Philip Hands wrote: > I thought I'd have a look through the d-i code for instances of: > > cat ... | > > to see if I could find any code with room for improvement, and came up > with the attached patch. Any comments before I commit these? (along > with relevant changelog entries, of course, and removing the comment > about removing a space from the disk name that's only there to annotate > the patch) I've had a quick look, but after only a few items I have some doubts... > - [ $(cat "$file" | wc -l) -gt 1 ] || continue > + [ $(wc -l "$file") -gt 1 ] || continue This is plain broken: $ cat massbuild.versions | wc -l 3 $ wc -l massbuild.versions 3 massbuild.versions I happen to know this one, but I bet there are there other commands that have subtle differences in output when using a file argument versus a pipe. > - for dir in $( (cat /tmp/mount.pre /tmp/mount.pre; mountpoints ) | \ > - sort -r | uniq -c | grep "^[[:space:]]*1[[:space:]]" | \ > - sed "s/^[[:space:]]*[0-9][[:space:]]//"); do > + for dir in $(mountpoints | sort -r /tmp/mount.pre /tmp/mount.pre - | uniq -u); do 1) I find the combination of using a pipe _and_ named files for sort unintuitive and less readable 2) Where did the grep and sed statements disappear to? After spotting these two I must admit I did not look at the rest. Cheers, FJP
Attachment:
signature.asc
Description: This is a digitally signed message part.