Re: [libvirt] [PATCH] nwfilters: Test suite for checking created firewall entries

On 04/07/2010 03:02 PM, Stefan Berger wrote: >> If this is a bash test, you need to update the #!. >> Otherwise, you have to give up on local variables. > > I'll change this to be a bash script. No point in making the test

> portable across shells. > >>> + tmpfile=`mktemp` >>> + tmpfile2=`mktemp` >> >> Not all systems have mktemp; is it worth adding fallback code in that

> case? > > Hm, two hardcoded files like '/tmp/nwfiltervmtest1' and > '/tmp/nwfiltervmtest2' could actually be a replacement, no? Yeah, but names that obvious are prone to DoS attacks. If you are assuming bash, you can at least use $RANDOM to get past the worst of the security hole in being that blatant. Or, as long as we are assuming bash and linux, you could just skip the test if mktemp doesn't exist.

>> My review stops here - shell is my area of expertise, but my Linux >> network filtering knowledge is sparse. > > Ok. Thanks for the review. Will adapt and re-post. Well, one of the joys of open source is that you can have multiple reviewers, each with different angles of expertise, with a cumulative review better than any one person could give.

-- Eric Blake eblake(a)redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org [attachment "signature.asc" deleted by Stefan Berger/Watson/IBM]