Am 23.01.2013 14:03, schrieb Peter Maydell: > On 23 January 2013 12:07, Andreas Färber <afaer...@suse.de> wrote: >> Consolidate model checking into a new arm_cpu_class_by_name(). >> >> If the name matches an existing type, also check whether that type is >> actually (a sub-type of) TYPE_ARM_CPU. >> >> This fixes, e.g., -cpu tmp105 asserting. >> >> Cc: qemu-stable <qemu-sta...@nongnu.org> >> Signed-off-by: Andreas Färber <afaer...@suse.de> >> --- >> target-arm/cpu.c | 17 +++++++++++++++++ >> target-arm/helper.c | 6 ++++-- >> 2 Dateien geändert, 21 Zeilen hinzugefügt(+), 2 Zeilen entfernt(-) >> >> diff --git a/target-arm/cpu.c b/target-arm/cpu.c >> index 07588a1..d85f251 100644 >> --- a/target-arm/cpu.c >> +++ b/target-arm/cpu.c >> @@ -201,6 +201,21 @@ void arm_cpu_realize(ARMCPU *cpu) >> >> /* CPU models */ >> >> +static ObjectClass *arm_cpu_class_by_name(const char *cpu_model) >> +{ >> + ObjectClass *oc; >> + >> + if (cpu_model == NULL) { >> + return NULL; >> + } > > explicit "== NULL" is kind of ugly; established style in > target-arm/ is "if (!cpu_model)..."
I consistently use !foo only if foo is bool. Any decent compiler will optimize this appropriately. It not being that way in helper.c most likely is a symptom of you replacing my patch with your initfn approach. ;) > >> + >> + oc = object_class_by_name(cpu_model); > > I note that the object_class_by_name() implementation returns > NULL for NULL input, though the documentation doesn't guarantee > it will... > >> + if (oc == NULL || object_class_dynamic_cast(oc, TYPE_ARM_CPU) == NULL) { >> + return NULL; >> + } >> + return oc; >> +} >> + >> static void arm926_initfn(Object *obj) >> { >> ARMCPU *cpu = ARM_CPU(obj); >> @@ -766,6 +781,8 @@ static void arm_cpu_class_init(ObjectClass *oc, void >> *data) >> >> acc->parent_reset = cc->reset; >> cc->reset = arm_cpu_reset; >> + >> + cc->class_by_name = arm_cpu_class_by_name; > > Is this a class method because the plan is that eventually > the code that instantiates the CPU object will become > generic rather than target specific? Yes, the plan as indicated in the CPUState realizefn series is to generalize cpu_init() so that it only needs to know which base type to operate on. I'm not yet sure how to handle CPU properties in a generic way, but said series got three or four targets into a generic QOM'ish form already. > >> } >> >> static void cpu_register(const ARMCPUInfo *info) >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index 37c34a1..4c29117 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -1262,12 +1262,14 @@ ARMCPU *cpu_arm_init(const char *cpu_model) >> { >> ARMCPU *cpu; >> CPUARMState *env; >> + ObjectClass *oc; >> static int inited = 0; >> >> - if (!object_class_by_name(cpu_model)) { >> + oc = cpu_class_by_name(TYPE_ARM_CPU, cpu_model); >> + if (oc == NULL) { >> return NULL; >> } >> - cpu = ARM_CPU(object_new(cpu_model)); >> + cpu = ARM_CPU(object_new(object_class_get_name(oc))); > > Do we really have to convert back to the char* type > name in order to instantiate an object given the class? Unless someone adds a new function, I fear so... internally TypeImpl is used as alternative but that's not really exposed so far. CC'ing Anthony. Cheers, Andreas > >> env = &cpu->env; >> env->cpu_model_str = cpu_model; >> arm_cpu_realize(cpu); >> -- >> 1.7.10.4 > > thanks > -- PMM > -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg