[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 18/58] isa: New isa_new(), isa_realize_and_unref() etc.
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v2 18/58] isa: New isa_new(), isa_realize_and_unref() etc. |
Date: |
Tue, 9 Jun 2020 10:26:11 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
On 6/9/20 10:19 AM, Philippe Mathieu-Daudé wrote:
> On 5/29/20 3:44 PM, Markus Armbruster wrote:
>> I'm converting from qdev_create()/qdev_init_nofail() to
>> qdev_new()/qdev_realize_and_unref(); recent commit "qdev: New
>> qdev_new(), qdev_realize(), etc." explains why.
>>
>> ISA devices use qdev_create() through isa_create() and
>> isa_try_create().
>>
>> Provide isa_new(), isa_try_new(), and isa_realize_and_unref() for
>> converting ISA devices.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> include/hw/isa/isa.h | 3 +++
>> hw/isa/isa-bus.c | 15 +++++++++++++++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h
>> index 02c2350274..3b6215fafe 100644
>> --- a/include/hw/isa/isa.h
>> +++ b/include/hw/isa/isa.h
>> @@ -105,6 +105,9 @@ MemoryRegion *isa_address_space(ISADevice *dev);
>> MemoryRegion *isa_address_space_io(ISADevice *dev);
>> ISADevice *isa_create(ISABus *bus, const char *name);
>> ISADevice *isa_try_create(ISABus *bus, const char *name);
>> +ISADevice *isa_new(const char *name);
>> +ISADevice *isa_try_new(const char *name);
>> +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp);
>> ISADevice *isa_create_simple(ISABus *bus, const char *name);
>>
>> ISADevice *isa_vga_init(ISABus *bus);
>> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
>> index 1c9d7e19ab..e6412d39b4 100644
>> --- a/hw/isa/isa-bus.c
>> +++ b/hw/isa/isa-bus.c
>> @@ -176,6 +176,16 @@ ISADevice *isa_try_create(ISABus *bus, const char *name)
>> return ISA_DEVICE(dev);
>> }
>>
>> +ISADevice *isa_new(const char *name)
>> +{
>> + return ISA_DEVICE(qdev_new(name));
>> +}
>> +
>> +ISADevice *isa_try_new(const char *name)
>> +{
>> + return ISA_DEVICE(qdev_try_new(name));
>
> We have:
>
> #define ISA_DEVICE(obj) \
> OBJECT_CHECK(ISADevice, (obj), TYPE_ISA_DEVICE)
>
> With:
>
> #define OBJECT_CHECK(type, obj, name) \
> ((type *)object_dynamic_cast_assert(OBJECT(obj), (name), \
> __FILE__, __LINE__, __func__))
>
> "If an invalid object is passed to this function, a run time
> assert will be generated."
Looking at object_dynamic_cast_assert() implementation, NULL is a
"valid" object...
>
> I'd expect isa_try_new() to return NULL if the type_name is not
> registered...
This is what is returned, as object_dynamic_cast_assert(NULL) = NULL.
This is very unclear (and not the first time I dig to understand that).
So this patch is logically correct.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>> +}
>> +
>> ISADevice *isa_create_simple(ISABus *bus, const char *name)
>> {
>> ISADevice *dev;
>> @@ -185,6 +195,11 @@ ISADevice *isa_create_simple(ISABus *bus, const char
>> *name)
>> return dev;
>> }
>>
>> +bool isa_realize_and_unref(ISADevice *dev, ISABus *bus, Error **errp)
>> +{
>> + return qdev_realize_and_unref(&dev->parent_obj, &bus->parent_obj, errp);
>> +}
>> +
>> ISADevice *isa_vga_init(ISABus *bus)
>> {
>> switch (vga_interface_type) {
>>
>