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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread 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@xxxxxxxxxx    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]