Am 02.12.2011 02:08, schrieb Anthony Liguori:
> On 12/01/2011 09:52 AM, Kevin Wolf wrote:
>> Am 30.11.2011 22:03, schrieb Anthony Liguori:
>>> qdev properties are settable only during construction and static to classes.
>>> This isn't flexible enough for QOM.
>>>
>>> This patch introduces a property interface for qdev that provides dynamic
>>> properties that are tied to objects, instead of classes. These properties
>>> are
>>> Visitor based instead of string based too.
>>>
>>> Signed-off-by: Anthony Liguori<aligu...@us.ibm.com>
>>> ---
>>> hw/qdev.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>> hw/qdev.h | 118
>>> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> qerror.c | 4 ++
>>> qerror.h | 3 ++
>>> 4 files changed, 224 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/hw/qdev.c b/hw/qdev.c
>>> index 106407f..ad2d44f 100644
>>> --- a/hw/qdev.c
>>> +++ b/hw/qdev.c
>>> @@ -390,12 +390,33 @@ void qdev_init_nofail(DeviceState *dev)
>>> }
>>> }
>>>
>>> +static void qdev_property_del_all(DeviceState *dev)
>>> +{
>>> + while (dev->properties) {
>>> + GSList *i = dev->properties;
>>> + DeviceProperty *prop = i->data;
>>> +
>>> + dev->properties = i->next;
>>> +
>>> + if (prop->release) {
>>> + prop->release(dev, prop->name, prop->opaque);
>>> + }
>>> +
>>> + g_free(prop->name);
>>> + g_free(prop->type);
>>> + g_free(prop);
>>> + g_free(i);
>>> + }
>>> +}
>>> +
>>> /* Unlink device from bus and free the structure. */
>>> void qdev_free(DeviceState *dev)
>>> {
>>> BusState *bus;
>>> Property *prop;
>>>
>>> + qdev_property_del_all(dev);
>>> +
>>> if (dev->state == DEV_STATE_INITIALIZED) {
>>> while (dev->num_child_bus) {
>>> bus = QLIST_FIRST(&dev->child_bus);
>>> @@ -962,3 +983,81 @@ char* qdev_get_fw_dev_path(DeviceState *dev)
>>>
>>> return strdup(path);
>>> }
>>> +
>>> +void qdev_property_add(DeviceState *dev, const char *name, const char
>>> *type,
>>> + DevicePropertyEtter *get, DevicePropertyEtter *set,
>>> + DevicePropertyRelease *release, void *opaque,
>>> + Error **errp)
>>
>> How about letting the caller pass in a DeviceProperty for improved
>> readability and usability? Instead of memorizing the order of currently
>> eight parameters (could probably become more in the future) you can use
>> proper C99 initializers then.
>
> Yeah, instead of taking a void *opaque, it could then just take the
> DeviceProperty and use container_of adding a good bit more type safety. I
> like
> it, thanks for the suggestion.
>
>>
>>> @@ -45,6 +82,7 @@ struct DeviceState {
>>> QTAILQ_ENTRY(DeviceState) sibling;
>>> int instance_id_alias;
>>> int alias_required_for_version;
>>> + GSList *properties;
>>> };
>>
>> Why GSList instead of qemu-queue.h macros that would provide type safety?
>
> You're clearly thwarting my attempts at slowly introducing GSList as a
> replacement for qemu-queue ;-)
>
> I really dislike qemu-queue. I think it's a whole lot more difficult to use
> in
> practice. The glib data structures are much more rich than qemu-queue.
qemu-queue.h is type safe, GSList is not. IMO that's a show stopper and
I can't understand why we even need to talk about it.
If you want to convince me of the opposite, it certainly needs more than
vague "easier to use" hand waving.
Kevin
