|
From: | Paolo Bonzini |
Subject: | Re: [Qemu-devel] [PULL 16/16] target-i386: Move KVM default-vendor hack to instance_init |
Date: | Sun, 09 Feb 2014 00:33:05 +0100 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.2.0 |
Il 08/02/2014 18:28, Andreas Färber ha scritto:
+ /* sysenter isn't supported in compatibility mode on AMD, + * syscall isn't supported in compatibility mode on Intel. + * Normally we advertise the actual CPU vendor, but you can + * override this using the 'vendor' property if you want to use + * KVM's sysenter/syscall emulation in compatibility mode and + * when doing cross vendor migration + */ + const char *vendor = def->vendor; + char host_vendor[CPUID_VENDOR_SZ + 1];Since when is it OK to declare variables in the middle of the block?
When the code looks better, it is OK since always: checkpatch.pl doesn't complain and -Wdeclaration-after-statement is not added to the compiler flags. Usually I prefer to split a function if I feel the need for variable declarations in the middle of a block. In this case, the variable is used till the end of the function, and a "fake" {...} block is worse than this.
Are you planning to fix that?
No, I am not.
Once again I note that a patch to a file under my maintenance was applied without my review - and promptly a style bug slipped through.
If you consider it a style bug, post a patch to add the -Wdeclaration-after-statement flag and/or to detect in checkpatch.pl. Until then, things are left to everyone's taste. AFAIU declarations after statements are discouraged but not prohibited.
I applied these three patches because they only affect KVM. Eduardo's other series "target-i386: Simplify kvm_cpu_fill_host() and kvm_check_features_against_host()" is also touching only target-i386/cpu.c and it's obviously KVM stuff.
Paolo
[Prev in Thread] | Current Thread | [Next in Thread] |