From: Igor Mammedov <imammedo@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
"Daniel P. Berrangé" <berrange@redhat.com>,
qemu-devel@nongnu.org, "Markus Armbruster" <armbru@redhat.com>,
"Marc-André Lureau" <marcandre.lureau@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"John Snow" <jsnow@redhat.com>
Subject: Re: [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset
Date: Wed, 21 Oct 2020 14:24:08 +0200 [thread overview]
Message-ID: <20201021142408.72d2b92f@redhat.com> (raw)
In-Reply-To: <20201009160122.1662082-4-ehabkost@redhat.com>
On Fri, 9 Oct 2020 12:01:13 -0400
Eduardo Habkost <ehabkost@redhat.com> wrote:
> The existing object_class_property_add_uint*_ptr() functions are
> not very useful, because they need a pointer to the property
> value, which can't really be provided before the object is
> created.
>
> Replace the pointer parameter in those functions with a
> `ptrdiff_t offset` parameter.
>
> Include a uint8 class property in check-qom-proplist unit tests,
> to ensure the feature is working.
Not sure I like approach, it's reinventing qdev pointer properties in QOM form.
I had an impression that Paolo wanted qdev pointer properties be gone
and replaced by something like link properties.
object_property_add_uintXX_ptr() were introduced as a quick hack,
when ACPI code generation was moved from Seabios, to avoid more
code shuffling in device models and adding more boiler plate in
form of custom setters/getters (the later didn't seem to bother
us everywhere else where we use object_[class_]property_add() ).
Then it spread little bit to another places.
I'd rather get rid of object_property_add_uintXX_ptr() API altogether
in favor of object_[class_]property_add() like it is used in other places
to handle intXX properties.
Adding helpers similar to object_property_add_bool() for intXX
could reduce boiler plate need for converting current instances of
_ptr(), and such helpers would also help with reducing boilerplate
for the rest of instances where object_[class_]property_add()
currently is used for dealing with integers.
> 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
> ---
> include/qom/object.h | 8 ++++----
> qom/object.c | 36 +++++++++++++++++++++++-------------
> tests/check-qom-proplist.c | 10 ++++++++--
> 3 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/include/qom/object.h b/include/qom/object.h
> index d378f13a11..1634294e4f 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1747,7 +1747,7 @@ ObjectProperty *object_property_add_uint8_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint8_ptr(ObjectClass *klass,
> const char *name,
> - const uint8_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> @@ -1768,7 +1768,7 @@ ObjectProperty *object_property_add_uint16_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint16_ptr(ObjectClass *klass,
> const char *name,
> - const uint16_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> @@ -1789,7 +1789,7 @@ ObjectProperty *object_property_add_uint32_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint32_ptr(ObjectClass *klass,
> const char *name,
> - const uint32_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> @@ -1810,7 +1810,7 @@ ObjectProperty *object_property_add_uint64_ptr(Object *obj, const char *name,
>
> ObjectProperty *object_class_property_add_uint64_ptr(ObjectClass *klass,
> const char *name,
> - const uint64_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags);
>
> /**
> diff --git a/qom/object.c b/qom/object.c
> index 17692ed5c3..bb32f5d3ad 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -2450,13 +2450,22 @@ static char *object_get_type(Object *obj, Error **errp)
> }
>
> typedef struct {
> - /* Pointer to property value */
> - void *ptr;
> + bool is_offset;
> + union {
> + /* Pointer to property value. Valid if is_offset=false */
> + void *ptr;
> + /* Offset in Object struct. Valid if is_offset=true */
> + ptrdiff_t offset;
> + };
> } PointerProperty;
>
> static void *pointer_property_get_ptr(Object *obj, PointerProperty *prop)
> {
> - return prop->ptr;
> + if (prop->is_offset) {
> + return (void *)obj + prop->offset;
> + } else {
> + return prop->ptr;
> + }
> }
>
> static void property_get_uint8_ptr(Object *obj, Visitor *v, const char *name,
> @@ -2573,10 +2582,11 @@ object_class_property_add_uint_ptr(ObjectClass *oc, const char *name,
> ObjectPropertyAccessor getter,
> ObjectPropertyAccessor setter,
> ObjectPropertyFlags flags,
> - void *ptr)
> + ptrdiff_t offset)
> {
> PointerProperty *prop = g_new0(PointerProperty, 1);
> - prop->ptr = ptr;
> + prop->is_offset = true;
> + prop->offset = offset;
> return object_class_property_add(oc, name, type,
> (flags & OBJ_PROP_FLAG_READ) ? getter : NULL,
> (flags & OBJ_PROP_FLAG_WRITE) ? setter : NULL,
> @@ -2597,13 +2607,13 @@ object_property_add_uint8_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint8_ptr(ObjectClass *klass, const char *name,
> - const uint8_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint8",
> property_get_uint8_ptr,
> property_set_uint8_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> ObjectProperty *
> @@ -2620,13 +2630,13 @@ object_property_add_uint16_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint16_ptr(ObjectClass *klass, const char *name,
> - const uint16_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint16",
> property_get_uint16_ptr,
> property_set_uint16_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> ObjectProperty *
> @@ -2643,13 +2653,13 @@ object_property_add_uint32_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name,
> - const uint32_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint32",
> property_get_uint32_ptr,
> property_set_uint32_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> ObjectProperty *
> @@ -2666,13 +2676,13 @@ object_property_add_uint64_ptr(Object *obj, const char *name,
>
> ObjectProperty *
> object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
> - const uint64_t *v,
> + ptrdiff_t offset,
> ObjectPropertyFlags flags)
> {
> return object_class_property_add_uint_ptr(klass, name, "uint64",
> property_get_uint64_ptr,
> property_set_uint64_ptr,
> - flags, (void *)v);
> + flags, offset);
> }
>
> typedef struct {
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 1b76581980..fba30c20b2 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -61,6 +61,7 @@ struct DummyObject {
> bool bv;
> DummyAnimal av;
> char *sv;
> + uint8_t u8v;
> };
>
> struct DummyObjectClass {
> @@ -141,6 +142,9 @@ static void dummy_class_init(ObjectClass *cls, void *data)
> &dummy_animal_map,
> dummy_get_av,
> dummy_set_av);
> + object_class_property_add_uint8_ptr(cls, "u8v",
> + offsetof(DummyObject, u8v),
> + OBJ_PROP_FLAG_READWRITE);
> }
>
>
> @@ -385,12 +389,14 @@ static void test_dummy_createlist(void)
> "bv", "yes",
> "sv", "Hiss hiss hiss",
> "av", "platypus",
> + "u8v", "42",
> NULL));
>
> g_assert(err == NULL);
> g_assert_cmpstr(dobj->sv, ==, "Hiss hiss hiss");
> g_assert(dobj->bv == true);
> g_assert(dobj->av == DUMMY_PLATYPUS);
> + g_assert_cmpint(dobj->u8v, ==, 42);
>
> g_assert(object_resolve_path_component(parent, "dummy0")
> == OBJECT(dobj));
> @@ -531,7 +537,7 @@ static void test_dummy_iterator(void)
> {
> const char *expected[] = {
> "type", /* inherited from TYPE_OBJECT */
> - "sv", "av", /* class properties */
> + "sv", "av", "u8v", /* class properties */
> "bv"}; /* instance property */
> Object *parent = object_get_objects_root();
> DummyObject *dobj = DUMMY_OBJECT(
> @@ -552,7 +558,7 @@ static void test_dummy_iterator(void)
>
> static void test_dummy_class_iterator(void)
> {
> - const char *expected[] = { "type", "av", "sv" };
> + const char *expected[] = { "type", "av", "sv", "u8v" };
> ObjectPropertyIterator iter;
> ObjectClass *klass = object_class_by_name(TYPE_DUMMY);
>
next prev parent reply other threads:[~2020-10-21 12:26 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-09 16:01 [PATCH 00/12] qom: Make all -object types use only class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 01/12] qom: Helpers for pointer properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 02/12] qom: Introduce PointerProperty struct Eduardo Habkost
2020-10-09 16:01 ` [PATCH 03/12] qom: Make object_class_property_add_uint*_ptr() get offset Eduardo Habkost
2020-10-09 17:24 ` Eric Blake
2020-10-09 17:31 ` Eduardo Habkost
2020-10-21 12:24 ` Igor Mammedov [this message]
2020-10-21 13:30 ` Eduardo Habkost
2020-10-22 5:06 ` Markus Armbruster
2020-10-22 21:34 ` Eduardo Habkost
2020-10-23 15:33 ` Igor Mammedov
2020-10-27 22:18 ` Eduardo Habkost
2020-10-28 15:22 ` Paolo Bonzini
2020-10-28 15:53 ` Igor Mammedov
2020-10-29 12:56 ` Eduardo Habkost
2020-10-29 13:37 ` Igor Mammedov
2020-10-09 16:01 ` [PATCH 04/12] sev: Use class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 05/12] rng: " Eduardo Habkost
2020-10-09 16:01 ` [PATCH 06/12] can_host: " Eduardo Habkost
2020-10-12 14:52 ` Pavel Pisa
2020-10-09 16:01 ` [PATCH 07/12] colo: " Eduardo Habkost
2020-10-09 16:01 ` [PATCH 08/12] netfilter: Reorder functions Eduardo Habkost
2020-10-09 16:01 ` [PATCH 09/12] netfilter: Use class properties Eduardo Habkost
2020-10-09 16:01 ` [PATCH 10/12] input: " Eduardo Habkost
2020-10-13 12:54 ` Gerd Hoffmann
2020-10-09 16:01 ` [PATCH 11/12] [RFC] qom: Property lock mechanism Eduardo Habkost
2020-10-09 16:01 ` [PATCH 12/12] [RFC] qom: Lock properties of all TYPE_USER_CREATABLE types Eduardo Habkost
2020-10-09 21:31 ` [PATCH] check-qom-proplist: Don't register instance props for user-creatable type Eduardo Habkost
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201021142408.72d2b92f@redhat.com \
--to=imammedo@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=ehabkost@redhat.com \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=marcandre.lureau@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).