Re: [Qemu-devel] [PATCH v16 13/23] target/rx: Fix cpu types and names

[Igor Mammedov <imammedo@redhat.com>]

On Fri, 31 May 2019 08:43:05 -0500
Richard Henderson <richard.henderson@linaro.org> wrote:

> There was confusion here about abstract classes and naming cpus.
> We had registered a concrete class named "-rxcpu".  This was put
> into the default cpu fields, and matched, so basic tests worked.
> However, no value for -cpu could ever match in rx_cpu_class_by_name.
> 
> Rename the base class to "rx-cpu" and make it abstract.  This
> matches what we do for most other targets.  Create a new concrete
> cpu with the name "rx62n-rx-cpu".

since it hasn't been merged yet, it would be better to squash this
fixup into 3/23

[...]

> diff --git a/target/rx/cpu.c b/target/rx/cpu.c
> index 3268077d08..41fe1de4bb 100644
> --- a/target/rx/cpu.c
> +++ b/target/rx/cpu.c
> @@ -74,13 +74,14 @@ static void rx_cpu_list_entry(gpointer data, gpointer user_data)
>      const char *typename = object_class_get_name(OBJECT_CLASS(data));
>      int len = strlen(typename) - strlen(RX_CPU_TYPE_SUFFIX);
>  
> -    qemu_printf("%.*s\n", len, typename);
> +    qemu_printf("  %.*s\n", len, typename);
>  }
>  
>  void rx_cpu_list(void)
>  {
> -    GSList *list;
> -    list = object_class_get_list_sorted(TYPE_RXCPU, false);
> +    GSList *list = object_class_get_list_sorted(TYPE_RX_CPU, false);
> +
> +    qemu_printf("Available CPUs:\n");
>      g_slist_foreach(list, rx_cpu_list_entry, NULL);
>      g_slist_free(list);
>  }
> @@ -88,15 +89,17 @@ void rx_cpu_list(void)
>  static ObjectClass *rx_cpu_class_by_name(const char cpu_model)
>  {
>      ObjectClass *oc;
> -    char *typename = NULL;
> +    char *typename;
>  
> -    typename = g_strdup_printf(RX_CPU_TYPE_NAME(""));
> +    typename = g_strdup_printf(RX_CPU_TYPE_NAME("%s"), cpu_model);
>      oc = object_class_by_name(typename);

in case of new cpu, I'd allow only typename as cpu_model

s/typename/cpu_model/
  
which is compatible with '-device' naming and QMP/monitor interfaces
that we support.

and I would not add other naming schemes /like adding suffix to cpu_model or .../
that  are existing in QEMU for legacy reasons.


> -    if (oc != NULL && object_class_is_abstract(oc)) {
> -        oc = NULL;
> -    }
> -
>      g_free(typename);
> +
> +    if (oc == NULL ||
> +        object_class_is_abstract(oc) ||
> +        !object_class_dynamic_cast(oc, TYPE_RX_CPU)) {
> +        return NULL;
> +    }
>      return oc;
>  }
>  
[...]