Re: [Qemu-trivial] [Qemu-devel] [PATCH] qdev property: cleanup

[Cao jin <caoj.fnst@cn.fujitsu.com>]

On 04/08/2016 07:17 PM, Markus Armbruster wrote:

>> - * Static properties access data in a struct.  The actual type of the
>> - * property and the field depends on the property type.
>> + * Static properties access data in a struct. The actual type of ObjectProperty
>> + * and the struct field depends on the @qtype type.
>>    */
>
> Not sure this part is an improvement.  What's wrong with the current text?
>

In a word: little hard for newbies like me to understand. (I think 
comments are for newbies). see my feeling about the comment:

As per my understanding, property has 2 kinds, former qdev property and 
the latest QOM property. For me, the original description is too 
ambiguous about 'property'.

original: *The actual type of the property and the field depends on the 
property type*

Using two same word 'property' is ambiguous and hard for newbie to 
distinguish. The 1st 'property' should mean a QOM property. and the 2nd 
'property', I think the original author`s meaning is: qdev property. 
But, what is the qdev property *type*? cannot find 'type' field in the 
definition except a *qtype*

struct Property {
     const char   *name;
     PropertyInfo *info;
     ptrdiff_t    offset;
     uint8_t      bitnr;
     QType        qtype;
     int64_t      defval;
     int          arrayoffset;
     PropertyInfo *arrayinfo;
     int          arrayfieldsize;
};

And *the actual type of the field* depends on the qtype, take bitmap 
field for example, bitmap field in a structure is always a *int*, but 
when convert to QOM property, it is treated as a *bool*, see 
DEFINE_PROP_BIT, DEFINE_PROP_BIT64, its qtype are QTYPE_QBOOL.

But I am little confused also now, I think my modification isn`t perfect
1. see how qdev_property_add_static invoke object_property_add, it pass 
prop->info->name as its QOM property type

2. when structure field is enum, QOM property will treat it as 
string(not depends on qtype now), see code:

else if (prop->info->enum_table) {
     object_property_set_str(obj, prop->info->enum_table[prop->defval],
                             prop->name, &error_abort);

I will do more analyse before v2.

-- 
Yours Sincerely,

Cao jin