[lug] Bletcherous pipe, FIXME!

[Zan Lynx]

Andrew Gilmore wrote:
> I discovered this nastiness in something I recommended in a benchmark, and before I got it published, thought I'd see if anyone cared to try to fix it...
> 
> The goal is to generate a sane default hosts.allow tcp wrappers only allowing hosts on the local networks.
> 
> printf "ALL: localhost" >> /etc/hosts.allow              
> for I in `route -n |tail -n +3 |sed -e 's/ */ /g'| cut -f1,3 -d ' '
>    --output-delimiter=/ | grep -vE "^(0|169)" |sort -n`; do
>    printf ", $I" >> /etc/hosts.allow;
> done
> echo >> /etc/hosts.allow
> 
> Ouch, I know.
> 
> Anyone for a game of bash golf? :-)

Right away I see three opportunities to make it better.

Use echo instead of printf.  Echo is generally a shell builtin.

Instead of using a for I in `` loop, put that pipeline in front of a 
while read x; do loop.

Instead of using >> /etc/hosts.allow inside the loop, put it after the 
done.  All the output of the loop will be redirected and the file opened 
only once.

Even better, put the whole thing into a subshell with ( ) and redirect 
the output of it all at once.

So it might look like this:

(
    echo -n "ALL: localhost"
    route -n | tail -n +3 | sed -e 's/  */ /g' \
       | cut -f1,3 -d ' ' --output-delimiter=/ \
       | grep -vE "^(0|169)" \
       | sort -n \
    | while read I
    do
       echo -n ", $I"
    done
) >> /etc/hosts.allow

Notice that I also discovered the sed was wrong.  It has to be two 
spaces and then the star.
-- 
Zan Lynx
zlynx at acm.org

"Knowledge is Power.  Power Corrupts.  Study Hard.  Be Evil."