Re: [PATCH v3 1/4] hw/isa: add function to check for existence of device by its type

[Liav Albani]

On 2/27/22 09:27, Ani Sinha wrote:
> On Sat, 26 Feb 2022, Liav Albani wrote:
>
> > This function enumerates all attached ISA devices in the machine, and
> > tries to compare a given device type name to the enumerated devices.
> > For example, this can help other code to determine if a i8042 controller
> > exists in the machine.
> >
> > Signed-off-by: Liav Albani <liavalb@gmail.com>
> > ---
> >   hw/isa/isa-bus.c     | 23 +++++++++++++++++++++++
> >   include/hw/isa/isa.h |  1 +
> >   2 files changed, 24 insertions(+)
> >
> > diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> > index 6c31398dda..663aa36d29 100644
> > --- a/hw/isa/isa-bus.c
> > +++ b/hw/isa/isa-bus.c
> > @@ -222,6 +222,29 @@ void isa_build_aml(ISABus *bus, Aml *scope)
> >       }
> >   }
> >
> > +bool isa_check_device_existence(const char *typename)
> > +{
> > +    /*
> > +     * If there's no ISA bus, we know for sure that the checked ISA device type
> > +     * doesn't exist in the machine.
> > +     */
> > +    if (isabus == NULL) {
> nit: I would do if (!isabus) instead to keep uniformity with other parts
> of the code.
Hmm, OK, I'll change it because it seems really fine to do that this way 
too :)
> > +        return false;
> > +    }
> > +
> > +    BusChild *kid;
> > +    ISADevice *dev;
> > +
> > +    QTAILQ_FOREACH(kid, &isabus->parent_obj.children, sibling) {
> > +        dev = ISA_DEVICE(kid->child);
> > +        const char *object_type = object_get_typename(OBJECT(dev));
> > +        if (object_type && strcmp(object_type, typename) == 0) {
> nit: I would do !strcmp() instead.
Hmm, OK, I'll change it because it seems really fine to do that this way 
too :)
> > +            return true;
> > +        }
> > +    }
> > +    return false;
> > +}
> > +
> >   static void isabus_dev_print(Monitor *mon, DeviceState *dev, int indent)
> >   {
> >       ISADevice *d = ISA_DEVICE(dev);
> > diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
> > index d4417b34b6..65f0c7e28c 100644
> > --- a/include/hw/isa/isa.h
> > +++ b/include/hw/isa/isa.h
> > @@ -99,6 +99,7 @@ IsaDma *isa_get_dma(ISABus *bus, int nchan);
> >   MemoryRegion *isa_address_space(ISADevice *dev);
> >   MemoryRegion *isa_address_space_io(ISADevice *dev);
> >   ISADevice *isa_new(const char *name);
> > +bool isa_check_device_existence(const char *typename);
> Please provide documentation for this function in line with other
> functions like isa_register_ioport() and isa_register_portio_list()  in
> the same header.

Ah, I see what you mean - I'll write short descriptive documentation 
like what there's for other functions :)

Thanks for the suggestions!

Best regards,
Liav