On Tue, Nov 17, 2020 at 2:45 AM Eduardo Habkost <ehabk...@redhat.com> wrote:
> Using QLitObject lets us get rid of most of the
> .set_default_value functions, and just use
> object_property_set_default() directly.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
> Changes v1 -> v2:
> * Instead of initializing defval to QLIT_QNULL
> by default, just check for QTYPE_NONE, to find out if .defval
> was explicitly set. This avoids extra complexity at
> set_prop_arraylen().
> ---
> include/hw/qdev-properties-system.h | 2 +-
> include/qom/field-property-internal.h | 4 ---
> include/qom/field-property.h | 26 ++++++++-----------
> include/qom/property-types.h | 19 ++++++--------
> hw/core/qdev-properties-system.c | 8 ------
> qom/field-property.c | 27 ++++++++++++++------
> qom/property-types.c | 36 ++++-----------------------
> 7 files changed, 42 insertions(+), 80 deletions(-)
>
> diff --git a/include/hw/qdev-properties-system.h
> b/include/hw/qdev-properties-system.h
> index 0ac327ae60..a586424a33 100644
> --- a/include/hw/qdev-properties-system.h
> +++ b/include/hw/qdev-properties-system.h
> @@ -65,7 +65,7 @@ extern const PropertyInfo qdev_prop_pcie_link_width;
>
> #define DEFINE_PROP_UUID(_name, _state, _field) \
> DEFINE_PROP(_name, _state, _field, qdev_prop_uuid, QemuUUID, \
> - .set_default = true)
> + .defval = QLIT_QSTR("auto"))
>
> #define DEFINE_PROP_AUDIODEV(_n, _s, _f) \
> DEFINE_PROP(_n, _s, _f, qdev_prop_audiodev, QEMUSoundCard)
> diff --git a/include/qom/field-property-internal.h
> b/include/qom/field-property-internal.h
> index a7b7e2b69d..9bc29e9b67 100644
> --- a/include/qom/field-property-internal.h
> +++ b/include/qom/field-property-internal.h
> @@ -15,10 +15,6 @@ void field_prop_set_enum(Object *obj, Visitor *v, const
> char *name,
>
> void field_prop_set_default_value_enum(ObjectProperty *op,
> const Property *prop);
> -void field_prop_set_default_value_int(ObjectProperty *op,
> - const Property *prop);
> -void field_prop_set_default_value_uint(ObjectProperty *op,
> - const Property *prop);
>
> void field_prop_get_int32(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp);
> diff --git a/include/qom/field-property.h b/include/qom/field-property.h
> index 0cb1fe2217..3cfd19cc14 100644
> --- a/include/qom/field-property.h
> +++ b/include/qom/field-property.h
> @@ -6,6 +6,7 @@
>
> #include "qom/object.h"
> #include "qapi/util.h"
> +#include "qapi/qmp/qlit.h"
>
> /**
> * struct Property: definition of a field property
> @@ -27,21 +28,8 @@ struct Property {
> const PropertyInfo *info;
> ptrdiff_t offset;
> uint8_t bitnr;
> - /**
> - * @set_default: true if the default value should be set from @defval,
> - * in which case @info->set_default_value must not be NULL
> - * (if false then no default value is set by the property system
> - * and the field retains whatever value it was given by
> instance_init).
> - */
> - bool set_default;
> - /**
> - * @defval: default value for the property. This is used only if
> @set_default
> - * is true.
> - */
> - union {
> - int64_t i;
> - uint64_t u;
> - } defval;
> + /** @defval: If not QTYPE_NONE, the default value for the property */
> + QLitObject defval;
> /* private: */
> int arrayoffset;
> const PropertyInfo *arrayinfo;
> @@ -61,7 +49,13 @@ struct PropertyInfo {
> const QEnumLookup *enum_table;
> /** @print: String formatting function, for the human monitor */
> int (*print)(Object *obj, Property *prop, char *dest, size_t len);
> - /** @set_default_value: Callback for initializing the default value */
> + /**
> + * @set_default_value: Optional callback for initializing the default
> value
> + *
> + * Most property types don't need to set this, as by default
> + * object_property_set_default() is called with the value at
> + * Property.defval.
> + */
> void (*set_default_value)(ObjectProperty *op, const Property *prop);
> /** @create: Optional callback for creation of property */
> ObjectProperty *(*create)(ObjectClass *oc, const char *name,
> diff --git a/include/qom/property-types.h b/include/qom/property-types.h
> index 3132ddafd9..869d1a993a 100644
> --- a/include/qom/property-types.h
> +++ b/include/qom/property-types.h
> @@ -5,6 +5,7 @@
> #define QOM_PROPERTY_TYPES_H
>
> #include "qom/field-property.h"
> +#include "qapi/qmp/qlit.h"
>
> extern const PropertyInfo prop_info_bit;
> extern const PropertyInfo prop_info_bit64;
> @@ -25,34 +26,29 @@ extern const PropertyInfo prop_info_link;
>
> #define PROP_SIGNED(_state, _field, _defval, _prop, _type, ...) \
> FIELD_PROP(_state, _field, _prop, _type, \
> - .set_default = true, \
> - .defval.i = (_type)_defval, \
> + .defval = QLIT_QNUM_INT((_type)_defval), \
> __VA_ARGS__)
>
> #define PROP_UNSIGNED(_state, _field, _defval, _prop, _type, ...) \
> FIELD_PROP(_state, _field, _prop, _type, \
> - .set_default = true, \
> - .defval.u = (_type)_defval, \
> + .defval = QLIT_QNUM_UINT((_type)_defval), \
> __VA_ARGS__)
>
> #define PROP_BIT(_state, _field, _bit, _defval, ...) \
> FIELD_PROP(_state, _field, prop_info_bit, uint32_t, \
> .bitnr = (_bit), \
> - .set_default = true, \
> - .defval.u = (bool)_defval, \
> + .defval = QLIT_QBOOL(_defval), \
> __VA_ARGS__)
>
> #define PROP_BIT64(_state, _field, _bit, _defval, ...) \
> FIELD_PROP(_state, _field, prop_info_bit64, uint64_t, \
> .bitnr = (_bit), \
> - .set_default = true, \
> - .defval.u = (bool)_defval, \
> + .defval = QLIT_QBOOL(_defval), \
> __VA_ARGS__)
>
> #define PROP_BOOL(_state, _field, _defval, ...) \
> FIELD_PROP(_state, _field, prop_info_bool, bool, \
> - .set_default = true, \
> - .defval.u = (bool)_defval, \
> + .defval = QLIT_QBOOL(_defval), \
> __VA_ARGS__)
>
> #define PROP_LINK(_state, _field, _type, _ptr_type, ...) \
> @@ -131,8 +127,7 @@ extern const PropertyInfo prop_info_link;
> _arrayfield, _arrayprop, _arraytype) \
> DEFINE_PROP((PROP_ARRAY_LEN_PREFIX _name), \
> _state, _field, prop_info_arraylen, uint32_t, \
> - .set_default = true, \
> - .defval.u = 0, \
> + .defval = QLIT_QNUM_UINT(0), \
> .arrayinfo = &(_arrayprop), \
> .arrayfieldsize = sizeof(_arraytype), \
> .arrayoffset = offsetof(_state, _arrayfield))
> diff --git a/hw/core/qdev-properties-system.c
> b/hw/core/qdev-properties-system.c
> index 8da68f076c..d9be5372f6 100644
> --- a/hw/core/qdev-properties-system.c
> +++ b/hw/core/qdev-properties-system.c
> @@ -570,7 +570,6 @@ const PropertyInfo qdev_prop_blocksize = {
> " and " MAX_BLOCK_SIZE_STR,
> .get = field_prop_get_size32,
> .set = set_blocksize,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> /* --- Block device error handling policy --- */
> @@ -768,7 +767,6 @@ const PropertyInfo qdev_prop_pci_devfn = {
> .print = print_pci_devfn,
> .get = field_prop_get_int32,
> .set = set_pci_devfn,
> - .set_default_value = field_prop_set_default_value_int,
> };
>
> /* --- pci host address --- */
> @@ -1080,16 +1078,10 @@ static void set_uuid(Object *obj, Visitor *v,
> const char *name, void *opaque,
> g_free(str);
> }
>
> -static void set_default_uuid_auto(ObjectProperty *op, const Property
> *prop)
> -{
> - object_property_set_default_str(op, UUID_VALUE_AUTO);
> -}
> -
> const PropertyInfo qdev_prop_uuid = {
> .name = "str",
> .description = "UUID (aka GUID) or \"" UUID_VALUE_AUTO
> "\" for random value (default)",
> .get = get_uuid,
> .set = set_uuid,
> - .set_default_value = set_default_uuid_auto,
> };
> diff --git a/qom/field-property.c b/qom/field-property.c
> index cb729626ce..9cb5ded41a 100644
> --- a/qom/field-property.c
> +++ b/qom/field-property.c
> @@ -47,6 +47,20 @@ static ObjectPropertyAccessor *field_prop_setter(const
> PropertyInfo *info)
> return info->set ? field_prop_set : NULL;
> }
>
> +static void field_prop_set_default_value(ObjectProperty *op,
> + Property *prop)
> +{
> + if (qlit_type(&prop->defval) == QTYPE_NONE) {
> + return;
> + }
> +
> + if (prop->info->set_default_value) {
> + prop->info->set_default_value(op, prop);
> + } else {
> + object_property_set_default(op, qobject_from_qlit(&prop->defval));
> + }
> +}
> +
> /*
> * Property release callback for dynamically-created properties:
> * We call the underlying element's property release hook, and
> @@ -83,11 +97,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, prop);
> + if (op->init) {
> + op->init(obj, op);
> }
>
> op->allow_set = allow_set;
> @@ -113,9 +125,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);
> diff --git a/qom/property-types.c b/qom/property-types.c
> index 4b3fe15b6a..fe96f1f49a 100644
> --- a/qom/property-types.c
> +++ b/qom/property-types.c
> @@ -28,8 +28,11 @@ void field_prop_set_enum(Object *obj, Visitor *v, const
> char *name,
> void field_prop_set_default_value_enum(ObjectProperty *op,
> const Property *prop)
> {
> - object_property_set_default_str(op,
> - qapi_enum_lookup(prop->info->enum_table, prop->defval.i));
> + QObject *defval = qobject_from_qlit(&prop->defval);
> + const char *str = qapi_enum_lookup(prop->info->enum_table,
> + qnum_get_int(qobject_to(QNum,
> defval)));
> + object_property_set_default_str(op, str);
> + qobject_unref(defval);
> }
>
> const PropertyInfo prop_info_enum = {
> @@ -80,17 +83,11 @@ static void prop_set_bit(Object *obj, Visitor *v,
> const char *name,
> bit_prop_set(obj, prop, value);
> }
>
> -static void set_default_value_bool(ObjectProperty *op, const Property
> *prop)
> -{
> - object_property_set_default_bool(op, prop->defval.u);
> -}
> -
> const PropertyInfo prop_info_bit = {
> .name = "bool",
> .description = "on/off",
> .get = prop_get_bit,
> .set = prop_set_bit,
> - .set_default_value = set_default_value_bool,
> };
>
> /* Bit64 */
> @@ -139,7 +136,6 @@ const PropertyInfo prop_info_bit64 = {
> .description = "on/off",
> .get = prop_get_bit64,
> .set = prop_set_bit64,
> - .set_default_value = set_default_value_bool,
> };
>
> /* --- bool --- */
> @@ -166,7 +162,6 @@ const PropertyInfo prop_info_bool = {
> .name = "bool",
> .get = get_bool,
> .set = set_bool,
> - .set_default_value = set_default_value_bool,
> };
>
> /* --- 8bit integer --- */
> @@ -189,23 +184,10 @@ static void set_uint8(Object *obj, Visitor *v, const
> char *name, void *opaque,
> visit_type_uint8(v, name, ptr, errp);
> }
>
> -void field_prop_set_default_value_int(ObjectProperty *op,
> - const Property *prop)
> -{
> - object_property_set_default_int(op, prop->defval.i);
> -}
> -
> -void field_prop_set_default_value_uint(ObjectProperty *op,
> - const Property *prop)
> -{
> - object_property_set_default_uint(op, prop->defval.u);
> -}
> -
> const PropertyInfo prop_info_uint8 = {
> .name = "uint8",
> .get = get_uint8,
> .set = set_uint8,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> /* --- 16bit integer --- */
> @@ -232,7 +214,6 @@ const PropertyInfo prop_info_uint16 = {
> .name = "uint16",
> .get = get_uint16,
> .set = set_uint16,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> /* --- 32bit integer --- */
> @@ -277,14 +258,12 @@ const PropertyInfo prop_info_uint32 = {
> .name = "uint32",
> .get = get_uint32,
> .set = set_uint32,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> const PropertyInfo prop_info_int32 = {
> .name = "int32",
> .get = field_prop_get_int32,
> .set = set_int32,
> - .set_default_value = field_prop_set_default_value_int,
> };
>
> /* --- 64bit integer --- */
> @@ -329,14 +308,12 @@ const PropertyInfo prop_info_uint64 = {
> .name = "uint64",
> .get = get_uint64,
> .set = set_uint64,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> const PropertyInfo prop_info_int64 = {
> .name = "int64",
> .get = get_int64,
> .set = set_int64,
> - .set_default_value = field_prop_set_default_value_int,
> };
>
> /* --- string --- */
> @@ -431,7 +408,6 @@ const PropertyInfo prop_info_size32 = {
> .name = "size",
> .get = field_prop_get_size32,
> .set = set_size32,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> /* --- support for array properties --- */
> @@ -495,7 +471,6 @@ const PropertyInfo prop_info_arraylen = {
> .name = "uint32",
> .get = get_uint32,
> .set = set_prop_arraylen,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> /* --- 64bit unsigned int 'size' type --- */
> @@ -522,7 +497,6 @@ const PropertyInfo prop_info_size = {
> .name = "size",
> .get = get_size,
> .set = set_size,
> - .set_default_value = field_prop_set_default_value_uint,
> };
>
> /* --- object link property --- */
> --
> 2.28.0
>
>
>
--
Marc-André Lureau
