On Tue, Nov 24, 2020 at 10:58:27AM +0100, Markus Armbruster wrote: > Eduardo Habkost <ehabk...@redhat.com> 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 <ehabk...@redhat.com> > > --- > > 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