Re: [Qemu-stable] [PATCH qom-cpu for-1.4 04/14] target-arm: Detect attempt to instantiate non-CPU type in cpu_init()

[Peter Maydell]

On 23 January 2013 12:07, Andreas Färber <address@hidden> 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 <address@hidden>
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  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)..."

> +
> +    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?

>  }
>
>  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?

>      env = &cpu->env;
>      env->cpu_model_str = cpu_model;
>      arm_cpu_realize(cpu);
> --
> 1.7.10.4

thanks
-- PMM