Re: [PATCH v8 3/9] machine: Improve is_cpu_type_supported()

[Gavin Shan]

Hi Phil,

On 12/4/23 09:20, Gavin Shan wrote:
> On 12/1/23 20:57, Philippe Mathieu-Daudé wrote:
> > On 29/11/23 05:20, Gavin Shan wrote:
> > > It's no sense to check the CPU type when mc->valid_cpu_types[0] is
> > > NULL.
> >
> > This case is a programming error, right? We should simply:
> >
> >   assert(!mc->valid_cpu_types || *mc->valid_cpu_types);
>
> Yes, assert() should be used instead. I will do this like below:
>
>      if (mc->valid_cpu_types && machine->cpu_type) {
>          assert(mc->valid_cpu_types[1] != NULL);

I meant assert(mc->valid_cpu_types[0] != NULL) and sorry for the confusion.

      if (mc->valid_cpu_types && machine->cpu_type) {
          assert(mc->valid_cpu_types[0] != NULL);
          :
      }

> > > So the check is skipped for this particular case. The constraint
> > > has been taken when the error messags are appended.
> > >
> > > A precise hint for the error message is given when mc->valid_cpu_types[0]
> > > is the only valid entry. Besides, enumeration on mc->valid_cpu_types[0]
> > > when we have mutiple valid entries there is avoided to increase the code
> > > readability, as suggested by Philippe Mathieu-Daudé.
> > >
> > > Signed-off-by: Gavin Shan <gshan@redhat.com>
> > > ---
> > >   hw/core/machine.c | 18 ++++++++++++------
> > >   1 file changed, 12 insertions(+), 6 deletions(-)

Thanks,
Gavin