qom/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: Jinliang Zheng <alexjlzheng@tencent.com>
Currently, object_initialize_with_type() calls object_class_property_init_all()
before initializing Object->properties. This may cause Object->properties to
still be NULL when we call object_property_add() on Object.
For exmaple, if we extend DEFINE_PROP_ARRAY() to a version with a default value
other than 0:
#define DEFINE_PROP_ARRAY_EXAMPLE(_name, _state, _field, \
_arrayfield, _arrayprop, _arraytype) \
DEFINE_PROP((PROP_ARRAY_LEN_PREFIX _name), \
_state, _field, qdev_prop_arraylen_virtio_net, \
uint32_t, \
.set_default = true, \
.defval.u = <non-zero>, \
.arrayinfo = &(_arrayprop), \
.arrayfieldsize = sizeof(_arraytype), \
.arrayoffset = offsetof(_state, _arrayfield))
We should have:
object_initialize_with_type
object_class_property_init_all
ObjectProperty->init() / object_property_init_defval
...
set_prop_arraylen
object_property_add
object_property_try_add
g_hash_table_insert(Object->properties) <- NULL
obj->properties = g_hash_table_new_full() <- initializing
This patch fixes the above problem by exchanging the order of Ojbect->properties
initialization and object_class_property_init_all().
Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
qom/object.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/qom/object.c b/qom/object.c
index 157a45c5f8..734b52f048 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -556,9 +556,9 @@ static void object_initialize_with_type(Object *obj, size_t size, TypeImpl *type
memset(obj, 0, type->instance_size);
obj->class = type->class;
object_ref(obj);
- object_class_property_init_all(obj);
obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal,
NULL, object_property_free);
+ object_class_property_init_all(obj);
object_init_with_type(obj, type);
object_post_init_with_type(obj, type);
}
--
2.41.1
On Sun, 15 Sept 2024 at 17:12, <alexjlzheng@gmail.com> wrote: > > From: Jinliang Zheng <alexjlzheng@tencent.com> > > Currently, object_initialize_with_type() calls object_class_property_init_all() > before initializing Object->properties. This may cause Object->properties to > still be NULL when we call object_property_add() on Object. > > For exmaple, if we extend DEFINE_PROP_ARRAY() to a version with a default value > other than 0: > #define DEFINE_PROP_ARRAY_EXAMPLE(_name, _state, _field, \ > _arrayfield, _arrayprop, _arraytype) \ > DEFINE_PROP((PROP_ARRAY_LEN_PREFIX _name), \ > _state, _field, qdev_prop_arraylen_virtio_net, \ > uint32_t, \ > .set_default = true, \ > .defval.u = <non-zero>, \ > .arrayinfo = &(_arrayprop), \ > .arrayfieldsize = sizeof(_arraytype), \ > .arrayoffset = offsetof(_state, _arrayfield)) > We should have: > object_initialize_with_type > object_class_property_init_all > ObjectProperty->init() / object_property_init_defval > ... > set_prop_arraylen There's no set_prop_arraylen function in the codebase. Which function do you mean here ? > object_property_add > object_property_try_add > g_hash_table_insert(Object->properties) <- NULL > obj->properties = g_hash_table_new_full() <- initializing > > This patch fixes the above problem by exchanging the order of Ojbect->properties > initialization and object_class_property_init_all(). So this happens only if the initializer for a class property attempts to add a property to the object, right? That seems quite niche, and it would be good to clarify that in the commit message. I do wonder if it's something we ever intended to support. In particular note that you cannot currently validly add a *class* property to the class in the initializer for a class property (because it would invalidate the iterator over the class properties). Do you have a more concrete example of something you want to do that you're currently running into this with? > Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com> > --- > qom/object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qom/object.c b/qom/object.c > index 157a45c5f8..734b52f048 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -556,9 +556,9 @@ static void object_initialize_with_type(Object *obj, size_t size, TypeImpl *type > memset(obj, 0, type->instance_size); > obj->class = type->class; > object_ref(obj); > - object_class_property_init_all(obj); > obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal, > NULL, object_property_free); > + object_class_property_init_all(obj); > object_init_with_type(obj, type); > object_post_init_with_type(obj, type); > } On the other hand doing the initialization in this order is certainly safe, so if it's all we need to do to handle class prop initializers adding object properties maybe it's fine to do so. thanks -- PMM
ping <alexjlzheng@gmail.com> 于2024年9月15日周日 22:53写道: > > From: Jinliang Zheng <alexjlzheng@tencent.com> > > Currently, object_initialize_with_type() calls object_class_property_init_all() > before initializing Object->properties. This may cause Object->properties to > still be NULL when we call object_property_add() on Object. > > For exmaple, if we extend DEFINE_PROP_ARRAY() to a version with a default value > other than 0: > #define DEFINE_PROP_ARRAY_EXAMPLE(_name, _state, _field, \ > _arrayfield, _arrayprop, _arraytype) \ > DEFINE_PROP((PROP_ARRAY_LEN_PREFIX _name), \ > _state, _field, qdev_prop_arraylen_virtio_net, \ > uint32_t, \ > .set_default = true, \ > .defval.u = <non-zero>, \ > .arrayinfo = &(_arrayprop), \ > .arrayfieldsize = sizeof(_arraytype), \ > .arrayoffset = offsetof(_state, _arrayfield)) > We should have: > object_initialize_with_type > object_class_property_init_all > ObjectProperty->init() / object_property_init_defval > ... > set_prop_arraylen > object_property_add > object_property_try_add > g_hash_table_insert(Object->properties) <- NULL > obj->properties = g_hash_table_new_full() <- initializing > > This patch fixes the above problem by exchanging the order of Ojbect->properties > initialization and object_class_property_init_all(). > > Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com> > --- > qom/object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qom/object.c b/qom/object.c > index 157a45c5f8..734b52f048 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -556,9 +556,9 @@ static void object_initialize_with_type(Object *obj, size_t size, TypeImpl *type > memset(obj, 0, type->instance_size); > obj->class = type->class; > object_ref(obj); > - object_class_property_init_all(obj); > obj->properties = g_hash_table_new_full(g_str_hash, g_str_equal, > NULL, object_property_free); > + object_class_property_init_all(obj); > object_init_with_type(obj, type); > object_post_init_with_type(obj, type); > } > -- > 2.41.1 >
© 2016 - 2024 Red Hat, Inc.