From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55435) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCHOi-0004Hw-4Q for qemu-devel@nongnu.org; Sat, 08 Feb 2014 18:33:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCHOa-00048u-GE for qemu-devel@nongnu.org; Sat, 08 Feb 2014 18:33:20 -0500 Received: from mail-ea0-x234.google.com ([2a00:1450:4013:c01::234]:40912) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCHOa-00048q-9V for qemu-devel@nongnu.org; Sat, 08 Feb 2014 18:33:12 -0500 Received: by mail-ea0-f180.google.com with SMTP id o10so2262580eaj.11 for ; Sat, 08 Feb 2014 15:33:11 -0800 (PST) Sender: Paolo Bonzini Message-ID: <52F6BEB1.2090606@redhat.com> Date: Sun, 09 Feb 2014 00:33:05 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1391445551-6561-1-git-send-email-pbonzini@redhat.com> <1391445551-6561-17-git-send-email-pbonzini@redhat.com> <52F6695A.7090801@suse.de> In-Reply-To: <52F6695A.7090801@suse.de> Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PULL 16/16] target-i386: Move KVM default-vendor hack to instance_init List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?ISO-8859-15?Q?Andreas_F=E4rber?= , qemu-devel@nongnu.org, Eduardo Habkost 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