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 <imamm...@redhat.com> > > Reviewed-by: Eduardo Habkost <ehabk...@redhat.com> > > Very minor style comment below. > >> --- >> v4: >> - rename x86cpu_vendor_words2str() to x86_cpu_vendor_words2str() >> Suggested-By: Andreas Färber <afaer...@suse.de> >> v3: >> -replace e[bcd]x arguments naming with vendor[123] >> -fix/swap vendor2 and vendor3 order >> Spotted-By: Eduardo Habkost <ehabk...@redhat.com> >> 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 >> >> +static void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1, >> + uint32_t vendor2, uint32_t vendor3)
Same nit here. >> +{ >> + int i; >> + for (i = 0; i < 4; i++) { >> + dst[i] = vendor1 >> (8 * i); >> + dst[i + 4] = vendor2 >> (8 * i); >> + dst[i + 8] = vendor3 >> (8 * i); >> + } >> + dst[CPUID_VENDOR_SZ] = '\0'; >> +} >> + >> /* feature flags taken from "Intel Processor Identification and the CPUID >> * Instruction" and AMD's "CPUID Specification". In cases of disagreement >> * between feature naming conventions, aliases may be added. >> @@ -1213,15 +1225,10 @@ static char *x86_cpuid_get_vendor(Object *obj, Error >> **errp) >> X86CPU *cpu = X86_CPU(obj); >> CPUX86State *env = &cpu->env; >> char *value; >> - int i; >> >> value = (char *)g_malloc(CPUID_VENDOR_SZ + 1); >> - for (i = 0; i < 4; i++) { >> - value[i ] = env->cpuid_vendor1 >> (8 * i); >> - value[i + 4] = env->cpuid_vendor2 >> (8 * i); >> - value[i + 8] = env->cpuid_vendor3 >> (8 * i); >> - } >> - value[CPUID_VENDOR_SZ] = '\0'; >> + x86_cpu_vendor_words2str(value, env->cpuid_vendor1, env->cpuid_vendor2, >> + env->cpuid_vendor3); > > I would add one extra space, so the argument in the second line are > aligned with the first argument from the previous line. 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. Andreas > >> return value; >> } >> >> -- >> 1.7.1 >> > -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg