| Message ID | 20241031155350.3240361-6-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | RFC: require error handling for dynamically created objects | expand |
On Thu, Oct 31, 2024 at 03:53:50PM +0000, Daniel P. Berrangé wrote: > Since object_new() will assert(), it should only be used in scenarios > where the caller knows exactly what type it is asking to be created, > and can thus be confident in avoiding abstract types. > > Enforce this by using a macro wrapper which types to paste "" to the > type name. This will generate a compile error if not passed a static > const string, forcing callers to use object_new_dynamic() instead. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > include/qom/object.h | 12 +++++++++++- > qom/object.c | 2 +- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/include/qom/object.h b/include/qom/object.h > index 8c2f3551c5..6a21cb6ca0 100644 > --- a/include/qom/object.h > +++ b/include/qom/object.h > @@ -637,7 +637,17 @@ Object *object_new_with_class(ObjectClass *klass, Error **errp); > * > * Returns: The newly allocated and instantiated object. > */ > -Object *object_new(const char *typename); > + > +/* > + * NB, object_new_helper is just an internal helper, wrapped by > + * the object_new() macro which prevents invokation unless given > + * a static, const string. > + * > + * Code should call object_new(), or object_new_dynamic(), not > + * object_new_helper(). > + */ > +Object *object_new_helper(const char *typename); Nit; personally I'd call it object_new_internal(). No strong opinions. > +#define object_new(typename) object_new_static(typename "") Interesting trick on const check.. I see why the test cases need change now. Feel free to ignore the comment there then.. Could be an improvement to enforce error checks on new dynamic allocations. This should be better than my patch 1 indeed. Thanks,
diff --git a/include/qom/object.h b/include/qom/object.h index 8c2f3551c5..6a21cb6ca0 100644 --- a/include/qom/object.h +++ b/include/qom/object.h @@ -637,7 +637,17 @@ Object *object_new_with_class(ObjectClass *klass, Error **errp); * * Returns: The newly allocated and instantiated object. */ -Object *object_new(const char *typename); + +/* + * NB, object_new_helper is just an internal helper, wrapped by + * the object_new() macro which prevents invokation unless given + * a static, const string. + * + * Code should call object_new(), or object_new_dynamic(), not + * object_new_helper(). + */ +Object *object_new_helper(const char *typename); +#define object_new(typename) object_new_static(typename "") /** * object_new_dynamic: diff --git a/qom/object.c b/qom/object.c index 1ed62dc2c9..36c1c82815 100644 --- a/qom/object.c +++ b/qom/object.c @@ -800,7 +800,7 @@ Object *object_new_with_class(ObjectClass *klass, Error **errp) return object_new_with_type(klass->type, errp); } -Object *object_new(const char *typename) +Object *object_new_helper(const char *typename) { TypeImpl *ti = type_get_by_name(typename);
Since object_new() will assert(), it should only be used in scenarios where the caller knows exactly what type it is asking to be created, and can thus be confident in avoiding abstract types. Enforce this by using a macro wrapper which types to paste "" to the type name. This will generate a compile error if not passed a static const string, forcing callers to use object_new_dynamic() instead. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/qom/object.h | 12 +++++++++++- qom/object.c | 2 +- 2 files changed, 12 insertions(+), 2 deletions(-)