[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [PATCH 2/3] qdev: support properties which d
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [PATCH 2/3] qdev: support properties which don't set a default value |
Date: |
Thu, 13 Jul 2017 20:25:10 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
Peter Maydell <address@hidden> writes:
> On 13 July 2017 at 18:10, Markus Armbruster <address@hidden> wrote:
>> Peter Maydell <address@hidden> writes:
>>
>>> On 12 July 2017 at 12:22, Markus Armbruster <address@hidden> wrote:
>>>> Peter Maydell <address@hidden> writes:
>>>>
>>>>> In some situations it's useful to have a qdev property which doesn't
>>>>> automatically set its default value when qdev_property_add_static is
>>>>> called (for instance when the default value is not constant).
>>>>>
>>>>> Support this by adding a flag to the Property struct indicating
>>>>> whether to set the default value. This replaces the existing test
>>>>> for whether the PorpertyInfo set_default_value function pointer is
>>>>
>>>> PropertyInfo
>>>>
>>>>> NULL, and we set the .set_default field to true for all those cases
>>>>> of struct Property which use a PropertyInfo with a non-NULL
>>>>> set_default_value, so behaviour remains the same as before.
>>>>>
>>>>> We define two new macros DEFINE_PROP_SIGNED_NODEFAULT and
>>>>> DEFINE_PROP_UNSIGNED_NODEFAULT, to cover the most plausible use cases
>>>>> of wanting to set an integer property with no default value.
>>>>
>>>> Your text makes me wonder what is supposed to set the default value
>>>> then. Glancing ahead to PATCH 3, it looks like method instance_init()
>>>> is. Can you think of a scenario where something else sets it?
>>>
>>> OK, proposed extra paragraph for commit message:
>>>
>>> This gives us the semantics of:
>>> * if .set_default is true and .info->set_default_value is not NULL,
>>> then .defval is used as the the default value of the property
>>
>> If I read your patch correctly, then this should be "if .set_default is
>> true, then .info->set_default_value() must not be null, and .defval is
>> used ..."
>
> Yes.
Update your comment and commit message accordingly, and you may add
Reviewed-by: Markus Armbruster <address@hidden>
- Re: [Qemu-arm] [PATCH 3/3] target/arm: Make Cortex-M3 and M4 default to 8 PMSA regions, (continued)
[Qemu-arm] [PATCH 2/3] qdev: support properties which don't set a default value, Peter Maydell, 2017/07/11