Re: [Qemu-devel] [PULL 16/16] target-i386: Move KVM default-vendor hack to instance_init

[Paolo Bonzini <pbonzini@redhat.com>]

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