qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jan Kiszka
Subject: Re: [Qemu-devel] [PATCH 27/28] sysbus: apic: ioapic: convert to QEMU Object Model
Date: Wed, 25 Jan 2012 15:23:51 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666

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: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]