Peter Maydell <peter.mayd...@linaro.org> writes: > On 13 July 2017 at 18:10, Markus Armbruster <arm...@redhat.com> wrote: >> Peter Maydell <peter.mayd...@linaro.org> writes: >> >>> On 12 July 2017 at 12:22, Markus Armbruster <arm...@redhat.com> wrote: >>>> Peter Maydell <peter.mayd...@linaro.org> 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 <arm...@redhat.com>