<tt>Eric Blake <eblake@redhat.com> wrote on 04/21/2010 10:59:35 AM: > On 04/20/2010 05:50 PM, Stefan Berger wrote: > > I've done a quick style review, but have not looked closely at the > locking algorithm to see if it is sane, so you'll have to wait for > another review (possibly by me, if I have a longer chunk of time to > spend on it) before you get an ack. > > > #define VIR_FROM_THIS VIR_FROM_NWFILTER > > > > +#define IFINDEX2STR(VARNAME, ifindex) \ > > + char VARNAME[20]; \ > > + snprintf(VARNAME, sizeof(VARNAME), "%d", ifindex); > > Use gnulib's intprops.h for INT_BUFSIZE_BOUND(ifindex) instead of > hard-coding an array of 20 bytes. At which point you could safely use > sprintf instead of snprintf, for a micro-optimization (although I'm > still fine seeing snprintf).</tt> <tt>I was looking for a #define like that. Thanks for pointing me to this one.</tt> <tt> > > > + > > +static bool threadsTerminate = false; > > Explicit initialization to false is not necessary; the language > guarantees 0-initialization of variables at file scope.</tt> <tt>I know. I will remove it.</tt> <tt> > > > @@ -77,6 +78,7 @@ virNWFilterTearNWFilter(virDomainNetDefP > > static inline void > > virNWFilterTearVMNWFilters(virDomainObjPtr vm) { > > int i; > > + > > for (i = 0; i < vm->def->nnets; i++) > > As long as you are making a cosmetic-only change here, does it make > sense to switch to 'unsigned int i' for the iterator?</tt> <tt>Also def->nnets is defined as 'int', so that's why I am using int also here.</tt> <tt>Regards,</tt> <tt> Stefan</tt> <tt> > > I didn't notice anything else in my rapid style read-through. > > -- > 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>