Re: [Qemu-devel] [PATCH 27/28] sysbus: apic: ioapic: convert to QEMU Object Model

[Anthony Liguori <anthony@codemonkey.ws>]

On 01/24/2012 04:06 PM, Jan Kiszka wrote:
> On 2012-01-24 22:53, Anthony Liguori wrote:
>> But I don't like this.  The problem is that the declarative syntax we
>> have doesn't distinguish between "not-specified" and
>> "zero-initialized".
>
> That's surely solvable.

Please try :-)

I've spent a good chunk of time mulling this over and could not find an 
acceptable solution.  I think the same is true for the GObject folks.

There is a declarative solution for this that I know of, a C++ class definition ;-)

>>   It's also tempting to just drop the if (ti->reset)
>> check but that means that you unconditionally override the base class
>> implementation even if it's not specified.
>
> That as well, just wrap this repeating pattern in a macro.

I'm fairly certain that would be hideous.

>> I don't see any tangible benefits to a declarative syntax except that it
>> makes it harder to get right because you have to write per-base class
>> initialization functions and it's very easy to get polymorphism wrong
>> there.
>
> Now you have to write way more derived class init functions - provided a
> base class is used more than once.

I don't understand why that's a problem.

>> We're not talking about a code size difference.  It's a wash in terms of
>> number of lines of code.
>
> We are. Code size will be smaller, so the number of lines. How much,
> that depends on the number of users per base class.

Compare:

static void my_device_class_init(ObjectClass *klass, void *data)
{
    DeviceClass *dc = DEVICE_CLASS(klass);
    dc->reset = my_device_reset;
}

static TypeInfo my_device_type_info = {
    .name = TYPE_MY_DEVICE,
    .parent = TYPE_DEVICE,
    .class_init = my_device_class_init,
};

static void register_devices(void)
{
    type_register_static(&my_device_type_info);
}

To:

static DeviceTypeInfo my_device_ops = {
     .reset = my_device_reset,
};

static TypeInfo my_device_type_info = {
     .name = TYPE_MY_DEVICE,
     .parent = TYPE_DEVICE,
     .class_init = device_generic_init,
     .class_data = my_device_ops,
};

static void register_devices(void)
{
    type_register_static(&my_device_type_info);
}

They're exactly the same size (16 lines).  If you embed TypeInfo into 
DeviceTypeInfo, and introduce a Device specific type registration function, then 
you could do:

static DeviceTypeInfo my_device_type_info = {
     .type.name = TYPE_MY_DEVICE,
     .type.parent = TYPE_PARENT_DEVICE,
     .reset = my_device_reset,
};

static void register_devices(void)
{
     device_type_register_static(&my_device_type_info);
}

Which admittedly saves 6 lines, but also is a big step backwards IMHO.  Now 
you've got a lot of one-off functions which means you loose the advantage of 
having everything work through the same infrastructure.  The likelihood that 
someone is going to do something whacky in device_type_register_static is high 
IMHO.  I think parent initialization (.type.name) is ugly.

Plus, if you do need a class_init function (if you want to save off the super 
class value for some function), then you need to revert back to style (1) anyway.

It'll never get better than this.  The 6 line difference is fixed and is mostly 
whitespace/curly brackets.

>>
>> Imperative allows you to explicit zero-initialize, accept the previous
>> version (from the base class), or override with a new value.
>
> None of this is impossible with a declarative approach.

The class_data method would allow for class initialization based on a 
declarative description so it will always be possible to add at some point if 
someone figures out the ultimate clever way to do it.

Regards,

Anthony Liguori

>
> Jan
>