<tt>Eric Blake <eblake@redhat.com> wrote on 03/18/2010 03:11:09 PM: > On 03/18/2010 09:16 AM, Stefan Berger wrote: > > This patch adds build support for the network filtering framework. > > > > Signed-off-by: Stefan Berger <stefanb@us.ibm.com> > > > > Some nits, but I'll let others more familiar with the process of API > expansion give an actual ack/nak review. > > > +NWFILTER_CONF_SOURCES = \ > > + $(NWFILTER_PARAM_CONF_SOURCES) \ > > + conf/nwfilter_conf.c conf/nwfilter_conf.h > > What's with the mix between tabs and spaces before the \?</tt> <tt>Fixed.</tt> <tt> > > Moreover, it seems a bit odd to hook up the Makefile support in 2/14 > when the new files don't exist until 12/14. But I guess that's okay as > long as the automake conditional that enables this block of code doesn't > trigger until the files exist.</tt> <tt>All patches up to 12/14 are necessary for anything to work. It's also possible that an earlier patch has a code-dependency on a later one, so ordering isn't quite that simple...</tt> <tt> > > > fi > > +if test "$with_nwfilter" = "yes" ; then > > + AC_DEFINE_UNQUOTED([WITH_NWFILTER], 1, [whether local network > filter management driver is available]) > > +fi > > You can use AC_DEFINE instead of AC_DEFINE_UNQUOTED here, since you > aren't doing any shell expansion on either WITH_NWFILTER or 1.</tt> <tt>Ok,</tt> <tt>Thanks and regards,</tt> <tt> Stefan</tt> <tt> > > -- > Eric Blake eblake@redhat.com +1-801-349-2682 > Libvirt virtualization library </tt><a href=http://libvirt.org/><tt>http://libvirt.org</tt></a><tt> > > [attachment "signature.asc" deleted by Stefan Berger/Watson/IBM] </tt>