Checking that the derived class is larger than the requested parent class saves us from some obvious mistakes, but as written, it does not catch all the cases; in particular, it is easy to forget to update a VIR_CLASS_NEW when changing the 'parent' member from virObject to virObjectLockabale, but where the size checks don't catch that. Add a parameter for one more layer of sanity checking. Note that I did NOT change the fact that we require derived classes to be larger (as the difference in size makes it easy to tell classes apart), which means that even if a derived class has no functionality to add (but rather exists for compiler-enforced type-safety), it must still include a dummy member. But I did fix the wording of the error message to match the code. Signed-off-by: Eric Blake <eblake@redhat.com> --- Here's hoping Coverity doesn't have a false-positive complaint about the error message being a potential dereference of NULL (the only time 'parent == NULL' is when 'parentsize == 0', based on the fact that our syntax checks forbid raw calls to virClassNew() except for "virObject" itself - but Coverity likely won't see that). src/util/virobject.h | 5 ++++- src/util/virobject.c | 8 +++++--- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/util/virobject.h b/src/util/virobject.h index d4ec943a43..757068fcc1 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -82,12 +82,15 @@ virClassPtr virClassForObjectRWLockable(void); */ # define VIR_CLASS_NEW(name, prnt) \ verify_expr(offsetof(name, parent) == 0, \ - (name##Class = virClassNew(prnt, #name, sizeof(name), name##Dispose))) + (name##Class = virClassNew(prnt, #name, sizeof(name), \ + sizeof(((name *)NULL)->parent), \ + name##Dispose))) virClassPtr virClassNew(virClassPtr parent, const char *name, size_t objectSize, + size_t parentSize, virObjectDisposeCallback dispose) VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(2); diff --git a/src/util/virobject.c b/src/util/virobject.c index 3b28331ba7..b4ee068cb2 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -78,6 +78,7 @@ virObjectOnceInit(void) if (!(virObjectClass = virClassNew(NULL, "virObject", sizeof(virObject), + 0, NULL))) return -1; @@ -159,6 +160,7 @@ virClassPtr virClassNew(virClassPtr parent, const char *name, size_t objectSize, + size_t parentSize, virObjectDisposeCallback dispose) { virClassPtr klass; @@ -167,10 +169,10 @@ virClassNew(virClassPtr parent, STRNEQ(name, "virObject")) { virReportInvalidNonNullArg(parent); return NULL; - } else if (parent && - objectSize <= parent->objectSize) { + } else if (objectSize <= parentSize || + parentSize != (parent ? parent->objectSize : 0)) { virReportInvalidArg(objectSize, - _("object size %zu of %s is smaller than parent class %zu"), + _("object size %zu of %s is not larger than parent class %zu"), objectSize, name, parent->objectSize); return NULL; } -- 2.20.1