On 11/01/2012 11:53 AM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
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.
ACK with the nit fixed. But, please, after the rebase (probably
tomorrow?), run the syntax-check and include all necessary changes to
make it pass (in case there are any), thanks.
Martin
P.S.: Again, if anyone has a look at the second patch in this series,
I'd appreciate that, since I'm not strong in perl. However if it
doesn't get a review, I'll try to do what I can.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list