Re: [PATCH v3 15/32] target/s390x: Use generic helper to show CPU model names

[Philippe Mathieu-Daudé]

On 8/9/23 10:04, Philippe Mathieu-Daudé wrote:
> On 8/9/23 01:44, Gavin Shan wrote:
> > On 9/7/23 18:20, David Hildenbrand wrote:
> > > On 07.09.23 02:35, Gavin Shan wrote:
> > > > For target/s390x, the CPU type name is always the combination of the
> > > > CPU modle name and suffix. The CPU model names have been correctly
> > > > shown in s390_print_cpu_model_list_entry() and create_cpu_model_list().
> > > >
> > > > Use generic helper cpu_model_from_type() to show the CPU model names
> > > > in the above two functions. Besides, we need validate the CPU class
> > > > in s390_cpu_class_by_name(), as other targets do.
> > > >
> > > > Signed-off-by: Gavin Shan <gshan@redhat.com>
> > > > ---
> > > >   target/s390x/cpu_models.c        | 18 +++++++++++-------
> > > >   target/s390x/cpu_models_sysemu.c |  9 ++++-----
> > > >   2 files changed, 15 insertions(+), 12 deletions(-)


> > > >   static gint s390_cpu_list_compare(gconstpointer a, gconstpointer b)
> > > > @@ -916,7 +915,12 @@ ObjectClass *s390_cpu_class_by_name(const char 
> > > > *name)
> > > >       oc = object_class_by_name(typename);
> > > >       g_free(typename);
> > > > -    return oc;
> > > > +    if (object_class_dynamic_cast(oc, TYPE_S390_CPU) &&
> > > > +        !object_class_is_abstract(oc)) {
> > > > +        return oc;
> > > > +    }
> > > > +
> > > > +    return NULL;
> > >
> > > Why is that change required?
> >
> > Good question. It's possible that other class's name conflicts with
> > CPU class's name. For example, class "abc-base-s390x-cpu" has been
> > registered for a misc class other than a CPU class. We don't want
> > s390_cpu_class_by_name() return the class for "abc-base-s390x-cpu".
> > Almost all other target does similar check.
>
> I agree with David there is some code smell here.
>
> IMO this check belong to cpu_class_by_name(), not to each impl.

Suggestion implemented here:
20230908112235.75914-1-philmd@linaro.org/">https://lore.kernel.org/qemu-devel/20230908112235.75914-1-philmd@linaro.org/