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

[Stefan Berger]

Eric Blake <eblake at 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).

I was looking for a #define like that. Thanks for pointing me to this one.

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

I know. I will remove it.

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

Also def->nnets is defined as 'int', so that's why I am using int also 
here.

Regards,
   Stefan


> 
> I didn't notice anything else in my rapid style read-through.
> 
> -- 
> Eric Blake   eblake at redhat.com    +1-801-349-2682
> Libvirt virtualization library http://libvirt.org
> 
> [attachment "signature.asc" deleted by Stefan Berger/Watson/IBM] 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20100421/72574a83/attachment-0001.htm>