On Fri, Oct 30, 2020 at 2:17 AM Eduardo Habkost <ehabkost@redhat.com> wrote:
> The array property registration code is hard to follow. Move the
> two steps into separate functions that have clear
> responsibilities.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Daniel P. Berrangé" <berrange@redhat.com>
> Cc: Eduardo Habkost <ehabkost@redhat.com>
> Cc: qemu-devel@nongnu.org
> ---
> hw/core/qdev-properties.c | 60 ++++++++++++++++++++++++++-------------
> 1 file changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 27c09255d7..1f06dfb5d5 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -588,6 +588,32 @@ typedef struct {
> ObjectPropertyRelease *release;
> } ArrayElementProperty;
>
> +/**
> + * Create ArrayElementProperty based on array length property
> + * @array_len_prop (which was previously defined using
> DEFINE_PROP_ARRAY()).
> + */
>
(some day we will have to clarify our API doc style, but not now ;)
+static ArrayElementProperty *array_element_new(Object *obj,
> + Property *array_len_prop,
> + const char *arrayname,
> + int index,
> + void *eltptr)
> +{
> + char *propname = g_strdup_printf("%s[%d]", arrayname, index);
> + ArrayElementProperty *arrayprop = g_new0(ArrayElementProperty, 1);
> + arrayprop->release = array_len_prop->arrayinfo->release;
> + arrayprop->propname = propname;
> + arrayprop->prop.info = array_len_prop->arrayinfo;
> + arrayprop->prop.name = propname;
> + /* This ugly piece of pointer arithmetic sets up the offset so
> + * that when the underlying get/set hooks call qdev_get_prop_ptr
> + * they get the right answer despite the array element not actually
> + * being inside the device struct.
> + */
> + arrayprop->prop.offset = eltptr - (void *)obj;
> + assert(qdev_get_prop_ptr(obj, &arrayprop->prop) == eltptr);
> + return arrayprop;
> +}
> +
> /* object property release callback for array element properties:
> * we call the underlying element's property release hook, and
> * then free the memory we allocated when we added the property.
> @@ -602,6 +628,18 @@ static void array_element_release(Object *obj, const
> char *name, void *opaque)
> g_free(p);
> }
>
> +static void object_property_add_array_element(Object *obj,
> + Property *array_len_prop,
> + ArrayElementProperty *prop)
> +{
> + object_property_add(obj, prop->prop.name,
> + prop->prop.info->name,
> + static_prop_getter(prop->prop.info),
> + static_prop_setter(prop->prop.info),
> + array_element_release,
> + prop);
> +}
> +
> static void set_prop_arraylen(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -641,25 +679,9 @@ static void set_prop_arraylen(Object *obj, Visitor
> *v, const char *name,
> */
> *arrayptr = eltptr = g_malloc0(*alenptr * prop->arrayfieldsize);
> for (i = 0; i < *alenptr; i++, eltptr += prop->arrayfieldsize) {
> - char *propname = g_strdup_printf("%s[%d]", arrayname, i);
> - ArrayElementProperty *arrayprop = g_new0(ArrayElementProperty, 1);
> - arrayprop->release = prop->arrayinfo->release;
> - arrayprop->propname = propname;
> - arrayprop->prop.info = prop->arrayinfo;
> - arrayprop->prop.name = propname;
> - /* This ugly piece of pointer arithmetic sets up the offset so
> - * that when the underlying get/set hooks call qdev_get_prop_ptr
> - * they get the right answer despite the array element not
> actually
> - * being inside the device struct.
> - */
> - arrayprop->prop.offset = eltptr - (void *)obj;
> - assert(qdev_get_prop_ptr(obj, &arrayprop->prop) == eltptr);
> - object_property_add(obj, propname,
> - arrayprop->prop.info->name,
> - static_prop_getter(arrayprop->prop.info),
> - static_prop_setter(arrayprop->prop.info),
> - array_element_release,
> - arrayprop);
> + ArrayElementProperty *elt_prop = array_element_new(obj, prop,
> arrayname,
> + i, eltptr);
> + object_property_add_array_element(obj, prop, elt_prop);
> }
> }
>
> --
> 2.28.0
>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
--
Marc-André Lureau