Re: [libvirt] [PATCH[ nwfilter: lock interface by its index

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).

> + > +static bool threadsTerminate = false; Explicit initialization to false is not necessary; the language guarantees 0-initialization of variables at file scope.

> @@ -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?

I didn't notice anything else in my rapid style read-through. -- 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]