From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48130) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TuoiT-0004Ky-Ld for qemu-devel@nongnu.org; Mon, 14 Jan 2013 13:25:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TuoiO-0000g8-C7 for qemu-devel@nongnu.org; Mon, 14 Jan 2013 13:25:01 -0500 Received: from mx1.redhat.com ([209.132.183.28]:39417) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TuoiO-0000ft-4Z for qemu-devel@nongnu.org; Mon, 14 Jan 2013 13:24:56 -0500 Date: Mon, 14 Jan 2013 19:24:52 +0100 From: Igor Mammedov Message-ID: <20130114192452.60145e25@thinkpad.mammed.net> In-Reply-To: <50F43CF3.9000405@suse.de> References: <1357870231-26762-1-git-send-email-imammedo@redhat.com> <1357870231-26762-5-git-send-email-imammedo@redhat.com> <20130111022847.GL10683@otherpad.lan.raisama.net> <50F43CF3.9000405@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 04/17] target-i386: add x86_cpu_vendor_words2str() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Andreas =?UTF-8?B?RsOkcmJlcg==?= Cc: Eduardo Habkost , qemu-devel@nongnu.org On Mon, 14 Jan 2013 18:14:27 +0100 Andreas F=C3=A4rber wrote: > Am 11.01.2013 03:28, schrieb Eduardo Habkost: > > On Fri, Jan 11, 2013 at 03:10:18AM +0100, Igor Mammedov wrote: > >> Make for() cycle reusable for the next patch > >> > >> Signed-off-by: Igor Mammedov > >=20 > > Reviewed-by: Eduardo Habkost > >=20 > > Very minor style comment below. > >=20 > >> --- > >> v4: > >> - rename x86cpu_vendor_words2str() to x86_cpu_vendor_words2str() > >> Suggested-By: Andreas F=C3=A4rber > >> v3: > >> -replace e[bcd]x arguments naming with vendor[123] > >> -fix/swap vendor2 and vendor3 order > >> Spotted-By: Eduardo Habkost > >> v2: > >> -place x86cpu_vendor_words2str() a bit earlier, before feature > >> arrays to avoid compile error when vendor property is converted > >> static property. > >> --- > >> target-i386/cpu.c | 21 ++++++++++++++------- > >> 1 files changed, 14 insertions(+), 7 deletions(-) > >> > >> diff --git a/target-i386/cpu.c b/target-i386/cpu.c > >> index c4ff761..64cc88f 100644 > >> --- a/target-i386/cpu.c > >> +++ b/target-i386/cpu.c > >> @@ -45,6 +45,18 @@ > >> #include "hw/apic_internal.h" > >> #endif > >> =20 > >> +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, > >> + uint32_t vendor2, uint32_t vendor= 3) >=20 > Same nit here. I'll resubmit fixed patch. Thanks! >=20 > >> +{ > >> + int i; > >> + for (i =3D 0; i < 4; i++) { > >> + dst[i] =3D vendor1 >> (8 * i); > >> + dst[i + 4] =3D vendor2 >> (8 * i); > >> + dst[i + 8] =3D vendor3 >> (8 * i); > >> + } > >> + dst[CPUID_VENDOR_SZ] =3D '\0'; > >> +} > >> + > >> /* feature flags taken from "Intel Processor Identification and the C= PUID > >> * Instruction" and AMD's "CPUID Specification". In cases of disagre= ement > >> * between feature naming conventions, aliases may be added. > >> @@ -1213,15 +1225,10 @@ static char *x86_cpuid_get_vendor(Object *obj,= Error **errp) > >> X86CPU *cpu =3D X86_CPU(obj); > >> CPUX86State *env =3D &cpu->env; > >> char *value; > >> - int i; > >> =20 > >> value =3D (char *)g_malloc(CPUID_VENDOR_SZ + 1); > >> - for (i =3D 0; i < 4; i++) { > >> - value[i ] =3D env->cpuid_vendor1 >> (8 * i); > >> - value[i + 4] =3D env->cpuid_vendor2 >> (8 * i); > >> - value[i + 8] =3D env->cpuid_vendor3 >> (8 * i); > >> - } > >> - value[CPUID_VENDOR_SZ] =3D '\0'; > >> + x86_cpu_vendor_words2str(value, env->cpuid_vendor1, env->cpuid_ve= ndor2, > >> + env->cpuid_vendor3); > >=20 > > I would add one extra space, so the argument in the second line are > > aligned with the first argument from the previous line. it was fixed in "[PATCH 04/17 v2] target-i386: add x86_cpu_vendor_words2str= ()" follow-up to original patch. >=20 > I can edit both in myself, but I won't manage to go through the whole > patchset(s) til tomorrow. 1-4 look fine so far. No need to edit. I'll re-submit fixed version in a moment. >=20 > Andreas >=20 > >=20 > >> return value; > >> } > >> =20 > >> --=20 > >> 1.7.1 > >> > >=20 >=20 >=20 > --=20 > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imend=C3=B6rffer; HRB 16746 AG N=C3= =BCrnberg >=20 --=20 Regards, Igor