On Tue, Nov 24, 2020 at 10:58:27AM +0100, Markus Armbruster wrote:
> Eduardo Habkost <[email protected]> writes:
>
> > Move code that sets the property default value to a separate
> > function, to reduce duplication and make refactoring easier.
> >
> > Signed-off-by: Eduardo Habkost <[email protected]>
> > ---
> > This is a new patch added in v3 of this series.
> > Hopefully, this will make the series easier to review.
> >
> > The field_prop_set_default_value() was added in v2 at
> > "qom: Use qlit to represent property defaults".
> > ---
> > qom/field-property.c | 24 ++++++++++++++++--------
> > 1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/qom/field-property.c b/qom/field-property.c
> > index cb729626ce..6a0df7c6ea 100644
> > --- a/qom/field-property.c
> > +++ b/qom/field-property.c
> > @@ -62,6 +62,17 @@ static void static_prop_release_dynamic_prop(Object
> > *obj, const char *name,
> > g_free(prop);
> > }
> >
> > +static void field_prop_set_default_value(ObjectProperty *op,
> > + Property *prop)
> > +{
> > + if (!prop->set_default) {
> > + return;
> > + }
> > +
> > + assert(prop->info->set_default_value);
> > + prop->info->set_default_value(op, prop);
> > +}
> > +
> > ObjectProperty *
> > object_property_add_field(Object *obj, const char *name,
> > Property *prop,
> > @@ -83,11 +94,9 @@ object_property_add_field(Object *obj, const char *name,
> > object_property_set_description(obj, name,
> > newprop->info->description);
> >
> > - if (newprop->set_default) {
> > - newprop->info->set_default_value(op, newprop);
> > - if (op->init) {
> > - op->init(obj, op);
> > - }
> > + field_prop_set_default_value(op, newprop);
> > + if (op->init) {
> > + op->init(obj, op);
> > }
>
> op->init() is now called even when !newprop->set_default. Why is that
> okay?
op->init() will be NULL if object_property_set_default() was not
called.
It's subtle, and worth mentioning in the commit message. I think
we should encapsulate op->init() behind a
object_property_reset_to_default_value() function.
>
> >
> > op->allow_set = allow_set;
> > @@ -113,9 +122,8 @@ object_class_property_add_field_static(ObjectClass *oc,
> > const char *name,
> > prop->info->release,
> > prop);
> > }
> > - if (prop->set_default) {
> > - prop->info->set_default_value(op, prop);
> > - }
> > +
> > + field_prop_set_default_value(op, prop);
> > if (prop->info->description) {
> > object_class_property_set_description(oc, name,
> > prop->info->description);
--
Eduardo
