Re: [libvirt] [PATCH v2 1/2] Remove spurious whitespace between function name & open brackets

On 11/01/2012 11:53 AM, Daniel P. Berrange wrote: > From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; > > The libvirt coding standard is to use 'function(...args...)' > instead of 'function (...args...)'. A non-trivial number of > places did not follow this rule and are fixed in this patch. > > Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; > --- It would be easier to review if it was split into few more commits, but I'm going for it anyway. [...] > diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c > index e691c04..ca8cd92 100644 > --- a/src/cpu/cpu_x86.c > +++ b/src/cpu/cpu_x86.c > @@ -548,16 +548,16 @@ x86VendorLoad(xmlXPathContextPtr ctxt, > } > > vendor->cpuid.function = 0; > - vendor->cpuid.ebx = (string[0] ) | > - (string[1] << 8) | > - (string[2] << 16) | > - (string[3] << 24); > - vendor->cpuid.edx = (string[4] ) | > - (string[5] << 8) | > - (string[6] << 16) | > - (string[7] << 24); > - vendor->cpuid.ecx = (string[8] ) | > - (string[9] << 8) | > + vendor->cpuid.ebx = (string[0]) | > + (string[1] << 8) | > + (string[2] << 16) | > + (string[3] << 24); > + vendor->cpuid.edx = (string[4]) | > + (string[5] << 8) | > + (string[6] << 16) | > + (string[7] << 24); > + vendor->cpuid.ecx = (string[8]) | > + (string[9] << 8) | > (string[10] << 16) | > (string[11] << 24); > Is this hunk (and similar) necessary? IMHO it doesn't violate the syntax and it's a bit nicer to read.

> diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c > index 350a8ec..2c0662f 100644 > --- a/src/nwfilter/nwfilter_dhcpsnoop.c > +++ b/src/nwfilter/nwfilter_dhcpsnoop.c > @@ -1084,7 +1084,7 @@ virNWFilterSnoopDHCPOpen(const char *ifname, virMacAddr *mac, > > virMacAddrFormat(mac, macaddr); > > - if (dir == PCAP_D_IN /* from VM */) { > + if (dir == PCAP_D_IN) /* from VM */) { This change fails the build, remove one of the brackets

[...] Just a question aside. We don't care about this kind of syntax? tools/virsh.h:197: bool (*handler) (vshControl *, const vshCmd *); I know it's even harder to algorithmically check this and I don't require it for this patch, I'm just curious.