| Message ID | 20200110153039.1379601-11-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Various qom & qdev enhancements | expand |
This patch caught my attention because of the typo in the function, but
I also noticed that get_default is never set to anything but
object_property_get_defval.
What do you think about removing the method and just relying on defval?
In practice there would be a new patch that squashes 7, 10 and the thing
after my signature.
Paolo
diff --git a/include/qom/object.h b/include/qom/object.h
index 1ea5c8c..035e41c 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -367,13 +367,6 @@ typedef void (ObjectPropertyRelease)(Object *obj,
*/
typedef void (ObjectPropertyInit)(Object *obj, ObjectProperty *prop);
-/**
- * ObjectPropertyGetDefault:
- *
- * Get an allocated string representation of the default value.
- */
-typedef char *(ObjectPropertyGetDefault)(ObjectProperty *prop);
-
struct ObjectProperty
{
gchar *name;
@@ -384,7 +377,6 @@ struct ObjectProperty
ObjectPropertyResolve *resolve;
ObjectPropertyRelease *release;
ObjectPropertyInit *init;
- ObjectPropertyGetDefault *get_default;
void *opaque;
QObject *defval;
};
diff --git a/qom/object.c b/qom/object.c
index 2464a9f..aa6cf19 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -1444,15 +1444,6 @@ int64_t object_property_get_int(Object *obj, const char *name,
return retval;
}
-char *object_property_get_default(ObjectProperty *prop)
-{
- if (!prop->get_default) {
- return NULL;
- }
-
- return prop->get_default(prop);
-}
-
static void object_property_init_defval(Object *obj, ObjectProperty *prop)
{
Visitor *v = qobject_input_visitor_new(prop->defval);
@@ -1463,8 +1454,12 @@ static void object_property_init_defval(Object *obj, ObjectProperty *prop)
visit_free(v);
}
-static char *object_property_get_defval(ObjectProperty *prop)
+char *object_property_get_default(ObjectProperty *prop)
{
+ if (!prop->defval) {
+ return NULL;
+ }
+
return qstring_free(qobject_to_json(prop->defval), TRUE);
}
@@ -1472,11 +1467,9 @@ static void object_property_set_default(ObjectProperty *prop, QObject *defval)
{
assert(!prop->defval);
assert(!prop->init);
- assert(!prop->get_default);
prop->defval = defval;
prop->init = object_property_init_defval;
- prop->get_default = object_property_get_defval;
}
void object_property_set_default_bool(ObjectProperty *prop, bool value)
@@ -2610,8 +2603,7 @@ void object_property_add_alias(Object *obj, const char *name,
goto out;
}
op->resolve = property_resolve_alias;
- if (target_prop->get_default) {
- op->get_default = target_prop->get_default;
+ if (target_prop->defval) {
op->defval = qobject_ref(target_prop->defval);
}
Hi On Thu, Jan 23, 2020 at 3:29 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > This patch caught my attention because of the typo in the function, but Ah! a french "défaut". > I also noticed that get_default is never set to anything but > object_property_get_defval. > > What do you think about removing the method and just relying on defval? > In practice there would be a new patch that squashes 7, 10 and the thing > after my signature. Indeed, we could remove the get_default callback. I can't find the reason I added it now. Are you resending the series then? > > Paolo > > diff --git a/include/qom/object.h b/include/qom/object.h > index 1ea5c8c..035e41c 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -367,13 +367,6 @@ typedef void (ObjectPropertyRelease)(Object *obj, > */ > typedef void (ObjectPropertyInit)(Object *obj, ObjectProperty *prop); > > -/** > - * ObjectPropertyGetDefault: > - * > - * Get an allocated string representation of the default value. > - */ > -typedef char *(ObjectPropertyGetDefault)(ObjectProperty *prop); > - > struct ObjectProperty > { > gchar *name; > @@ -384,7 +377,6 @@ struct ObjectProperty > ObjectPropertyResolve *resolve; > ObjectPropertyRelease *release; > ObjectPropertyInit *init; > - ObjectPropertyGetDefault *get_default; > void *opaque; > QObject *defval; > }; > diff --git a/qom/object.c b/qom/object.c > index 2464a9f..aa6cf19 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1444,15 +1444,6 @@ int64_t object_property_get_int(Object *obj, const char *name, > return retval; > } > > -char *object_property_get_default(ObjectProperty *prop) > -{ > - if (!prop->get_default) { > - return NULL; > - } > - > - return prop->get_default(prop); > -} > - > static void object_property_init_defval(Object *obj, ObjectProperty *prop) > { > Visitor *v = qobject_input_visitor_new(prop->defval); > @@ -1463,8 +1454,12 @@ static void object_property_init_defval(Object *obj, ObjectProperty *prop) > visit_free(v); > } > > -static char *object_property_get_defval(ObjectProperty *prop) > +char *object_property_get_default(ObjectProperty *prop) > { > + if (!prop->defval) { > + return NULL; > + } > + > return qstring_free(qobject_to_json(prop->defval), TRUE); > } > > @@ -1472,11 +1467,9 @@ static void object_property_set_default(ObjectProperty *prop, QObject *defval) > { > assert(!prop->defval); > assert(!prop->init); > - assert(!prop->get_default); > > prop->defval = defval; > prop->init = object_property_init_defval; > - prop->get_default = object_property_get_defval; > } > > void object_property_set_default_bool(ObjectProperty *prop, bool value) > @@ -2610,8 +2603,7 @@ void object_property_add_alias(Object *obj, const char *name, > goto out; > } > op->resolve = property_resolve_alias; > - if (target_prop->get_default) { > - op->get_default = target_prop->get_default; > + if (target_prop->defval) { > op->defval = qobject_ref(target_prop->defval); > } > > >
On 23/01/20 12:39, Marc-André Lureau wrote: > Hi > > On Thu, Jan 23, 2020 at 3:29 PM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> This patch caught my attention because of the typo in the function, but > > Ah! a french "défaut". I suspected that. :) >> I also noticed that get_default is never set to anything but >> object_property_get_defval. >> >> What do you think about removing the method and just relying on defval? >> In practice there would be a new patch that squashes 7, 10 and the thing >> after my signature. > > Indeed, we could remove the get_default callback. I can't find the > reason I added it now. > > Are you resending the series then? I have already sent a pull request. In the end even object_property_get_default was only used once so I just inlined it and dropped patch 7. Paolo
diff --git a/include/qom/object.h b/include/qom/object.h index 9f52bc365b..fb133d693f 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -386,6 +386,7 @@ struct ObjectProperty ObjectPropertyInit *init; ObjectPropertyGetDefault *get_default; void *opaque; + QObject *defval; }; /** @@ -1063,6 +1064,42 @@ ObjectProperty *object_class_property_add(ObjectClass *klass, const char *name, ObjectPropertyRelease *release, void *opaque, Error **errp); +/** + * object_property_set_defaut_bool: + * @prop: the property to set + * @value: the value to be written to the property + * + * Set the property default value. + */ +void object_property_set_defaut_bool(ObjectProperty *prop, bool value); + +/** + * object_property_set_defaut_str: + * @prop: the property to set + * @value: the value to be written to the property + * + * Set the property default value. + */ +void object_property_set_defaut_str(ObjectProperty *prop, const char *value); + +/** + * object_property_set_defaut_int: + * @prop: the property to set + * @value: the value to be written to the property + * + * Set the property default value. + */ +void object_property_set_defaut_int(ObjectProperty *prop, int64_t value); + +/** + * object_property_set_defaut_uint: + * @prop: the property to set + * @value: the value to be written to the property + * + * Set the property default value. + */ +void object_property_set_defaut_uint(ObjectProperty *prop, uint64_t value); + /** * object_property_find: * @obj: the object diff --git a/qom/object.c b/qom/object.c index a4c7bb01e6..aef7e64b5c 100644 --- a/qom/object.c +++ b/qom/object.c @@ -19,8 +19,10 @@ #include "qapi/visitor.h" #include "qapi/string-input-visitor.h" #include "qapi/string-output-visitor.h" +#include "qapi/qobject-input-visitor.h" #include "qapi/qapi-builtin-visit.h" #include "qapi/qmp/qerror.h" +#include "qapi/qmp/qjson.h" #include "trace.h" /* TODO: replace QObject with a simpler visitor to avoid a dependency @@ -264,6 +266,10 @@ static void object_property_free(gpointer data) { ObjectProperty *prop = data; + if (prop->defval) { + qobject_unref(prop->defval); + prop->defval = NULL; + } g_free(prop->name); g_free(prop->type); g_free(prop->description); @@ -1438,6 +1444,52 @@ char *object_property_get_default(ObjectProperty *prop) return prop->get_default(prop); } +static void object_property_init_defval(Object *obj, ObjectProperty *prop) +{ + Visitor *v = qobject_input_visitor_new(prop->defval); + + assert(prop->set != NULL); + prop->set(obj, v, prop->name, prop->opaque, &error_abort); + + visit_free(v); +} + +static char *object_property_get_defval(ObjectProperty *prop) +{ + return qstring_free(qobject_to_json(prop->defval), TRUE); +} + +static void object_property_set_defaut(ObjectProperty *prop, QObject *defval) +{ + assert(!prop->defval); + assert(!prop->init); + assert(!prop->get_default); + + prop->defval = defval; + prop->init = object_property_init_defval; + prop->get_default = object_property_get_defval; +} + +void object_property_set_defaut_bool(ObjectProperty *prop, bool value) +{ + object_property_set_defaut(prop, QOBJECT(qbool_from_bool(value))); +} + +void object_property_set_defaut_str(ObjectProperty *prop, const char *value) +{ + object_property_set_defaut(prop, QOBJECT(qstring_from_str(value))); +} + +void object_property_set_defaut_int(ObjectProperty *prop, int64_t value) +{ + object_property_set_defaut(prop, QOBJECT(qnum_from_int(value))); +} + +void object_property_set_defaut_uint(ObjectProperty *prop, uint64_t value) +{ + object_property_set_defaut(prop, QOBJECT(qnum_from_uint(value))); +} + void object_property_set_uint(Object *obj, uint64_t value, const char *name, Error **errp) { @@ -2549,6 +2601,10 @@ void object_property_add_alias(Object *obj, const char *name, goto out; } op->resolve = property_resolve_alias; + if (target_prop->get_default) { + op->get_default = target_prop->get_default; + op->defval = qobject_ref(target_prop->defval); + } object_property_set_description(obj, op->name, target_prop->description,
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/qom/object.h | 37 +++++++++++++++++++++++++++++ qom/object.c | 56 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+)