Mark Cave-Ayland <[email protected]> writes: > On 17/09/2026 09:52, Marc-André Lureau wrote: > >> Hi >> On Wed, Sep 2, 2026 at 6:42 PM Mark Cave-Ayland >> <[email protected]> wrote: >>> >>> This is eventually intended to be a replacement for >>> object_property_add_alias() >>> which uses an object property instead of a class property. >>> >>> With the advent of class properties, it is possible that QOM may attempt to >>> set or retrieve the value of an unset alias property. Update the existing >>> property_get_alias() and property_set_alias() functions to use the null >>> visitor >>> if the alias target has not been set, and property_resolve_alias() to return >>> NULL for the same case. >>> >>> This ensures that unset class alias properties accessed e.g. via the >>> monitor do >>> not cause QEMU to crash. >>> >>> Signed-off-by: Mark Cave-Ayland <[email protected]> >>> --- >>> include/qom/object.h | 25 +++++++++++ >>> qom/object.c | 100 +++++++++++++++++++++++++++++++++++++------ >>> 2 files changed, 113 insertions(+), 12 deletions(-) >>> >>> diff --git a/include/qom/object.h b/include/qom/object.h >>> index e24f0a2b2d..96c76975ef 100644 >>> --- a/include/qom/object.h >>> +++ b/include/qom/object.h >>> @@ -2263,6 +2263,11 @@ ObjectProperty >>> *object_class_static_property_add_uint64_ptr(ObjectClass *klass, >>> const uint64_t *v, >>> ObjectPropertyFlags flags); >>> >>> +typedef enum { >>> + /* private */ >>> + OBJ_PROP_ALIAS_CLASS = 0x1, >>> +} ObjectPropertyAliasFlags; >>> + >>> /** >>> * object_property_add_alias: >>> * @obj: the object to add a property to >>> @@ -2283,6 +2288,26 @@ ObjectProperty >>> *object_class_static_property_add_uint64_ptr(ObjectClass *klass, >>> ObjectProperty *object_property_add_alias(Object *obj, const char *name, >>> Object *target_obj, const char >>> *target_name); >>> >>> +/** >>> + * object_class_property_add_alias: >>> + * @klass: the object class to add a property to >>> + * @name: the name of the property >>> + * @offset: the offset from the object instance where the object alias is >>> + * stored >>> + * @target_type: QOM type we expect the alias to resolve to >>> + * @target_name: the name of the property on the forwarded object >>> + * >>> + * Add an alias for a property on an object. This function will add a >>> property >>> + * of the same type as the forwarded property. >>> + * >>> + * Returns: The newly added property on success, or %NULL on failure. >> actually, it assert() on error and never returns NULL. > > I was able to reproduce this by attempting to add a class alias property with > the same name again, and then started to look at other related functions do > to understand what the behaviour should be. There seems to be a number of > existing issues here: > > - object_property_try_add() returns NULL in the case a property cannot > be created (which seems to be only when an attempt to make add a > property that already exists). However since object_property_add() > always passes &error_abort, then QEMU terminates immediately without > returning the NULL.
Convention: when we have a pair of functions FOO_try_BAR() and FOO_BAR(), the former can fail and the latter cannot. The latter is useful when failure would be a programming error. When FOO_try_BAR() sets an Error on failure, FOO_BAR() is a thin wrapper that passes &error_abort. We pass &error_abort when failure would be a programming error. It's more concise than "call then assert it didn't fail". Concise is good. The wrapper makes calls that cannot fail even more concise. Worth having only when there are plenty of callers that profit from that. Someone decided this is the case for object_property_try_add() / object_property_add(). > - object_class_property_add() will currently assert() if an attempt is > made to add a duplicate property: it is fairly trivial to update it to > return the same error that object_property_add() does, however it is > interesting to note that the kernel-doc states that > error_setg(&error_abort, ...) should NOT be used. However in this case > it appears to return a genuinely helpful message which I think is > worth keeping. The only difference between object_class_property_add() and object_property_add() should be where the property is recorded (class vs. object). In particular, they should treat failure the same way. This may make object_class_property_try_add() necessary. > - There are several places in qom/object.c that do not check the > return value of object_property_add() / object_class_property_add() > such as object_add_link_prop(), object_property_add_alias() which > could potentially dereference a NULL pointer. object_property_add() is specified not to fail, i.e. not to return null. It's a programming error if it does. Letting the program crash on programming errors by dereferencing a null pointer is just fine. We can dress up the crash with assert(obj), but I wouldn't bother. Use object_property_add() when failure is a programming error. Use object_property_try_add() and handle the error when it's not. > It feels like the assumption is actually that these functions will > &error_abort rather than returning NULL as indicated by the kernel-doc. Any > thoughts on what we should do here? Ideally we want the same behaviour > between the object_property*() and object_class_property*() functions to aid > the transition. Hope this helps! [...]
