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

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2526 bytes --]

On 2012-01-25 15:00, Anthony Liguori wrote:
> On 01/25/2012 02:37 AM, Jan Kiszka wrote:
>> On 2012-01-25 00:03, Anthony Liguori wrote:
>>> 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,
>>
>> And if you introduce some
>>
>> #define TYPE_UNIMPLEMENTED (void *)&dummy_variable
> 
> (void *) isn't compatible with integers or function pointers (at least
> not in a portable way).

The former is true, the latter not. However, if you use pointers in the
TypeInfo structs consistently (const int * etc.), this issue should be
solvable as well.

> 
> You would need to use type specific unimplemented mechanisms.
> 
>>
>> you can easily express
>>
>> [.field = NULL]         =>  use parent
>>   .field = UNIMPLEMENTED =>  don't run any handler
>>
>>> };
>>>
>>> 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.
>>
>> Can't follow. Four lines
>>
>> [static inline] void device_type_register_static(DeviceTypeInfo *dt)
>> {
>>      type_register_static(&dt->type);
>> }
>>
>> are neither ugly nor complex. Rather, this helps concentrating effort at
>> _central_ places instead of decentralizing and duplicating lines like in
>> your imperative approach. That's the whole point: keep the common case
>> (derived classes) compact.
> 
> Send a patch.  The infrastructure available should be enough to do
> whatever you need to.
> 
> I'm not being dismissive, I've already spent a lot of time trying to
> make it work and have convinced myself that it isn't workable.
> 
> If you can show a mechanism that works, I don't mind scripting a mass
> conversion.  I'm also fairly confident that a workable solution is going
> to be better than what we have.

Well, I'm surely not happy having to go through all of the devices
again, even if scripted (scripts tend to neglect some coding style rules
e.g.). But if this change is in a hurry and my concern is the only one,
then let it be.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]