Alter to use more recent formatting guidelines Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virobject.c | 64 ++++++++++++++++++++++++++++++++++------------------ src/util/virobject.h | 59 ++++++++++++++++++++++++++++++++---------------- 2 files changed, 82 insertions(+), 41 deletions(-) diff --git a/src/util/virobject.c b/src/util/virobject.c index 51876b9..792685b 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -52,7 +52,8 @@ static virClassPtr virObjectLockableClass; static void virObjectLockableDispose(void *anyobj); -static int virObjectOnceInit(void) +static int +virObjectOnceInit(void) { if (!(virObjectClass = virClassNew(NULL, "virObject", @@ -77,7 +78,8 @@ VIR_ONCE_GLOBAL_INIT(virObject); * * Returns the class instance for the base virObject type */ -virClassPtr virClassForObject(void) +virClassPtr +virClassForObject(void) { if (virObjectInitialize() < 0) return NULL; @@ -91,7 +93,8 @@ virClassPtr virClassForObject(void) * * Returns the class instance for the virObjectLockable type */ -virClassPtr virClassForObjectLockable(void) +virClassPtr +virClassForObjectLockable(void) { if (virObjectInitialize() < 0) return NULL; @@ -116,10 +119,11 @@ virClassPtr virClassForObjectLockable(void) * * Returns a new class instance */ -virClassPtr virClassNew(virClassPtr parent, - const char *name, - size_t objectSize, - virObjectDisposeCallback dispose) +virClassPtr +virClassNew(virClassPtr parent, + const char *name, + size_t objectSize, + virObjectDisposeCallback dispose) { virClassPtr klass; @@ -162,8 +166,9 @@ virClassPtr virClassNew(virClassPtr parent, * * Return true if @klass is derived from @parent, false otherwise */ -bool virClassIsDerivedFrom(virClassPtr klass, - virClassPtr parent) +bool +virClassIsDerivedFrom(virClassPtr klass, + virClassPtr parent) { while (klass) { if (klass->magic == parent->magic) @@ -186,7 +191,8 @@ bool virClassIsDerivedFrom(virClassPtr klass, * * Returns the new object */ -void *virObjectNew(virClassPtr klass) +void * +virObjectNew(virClassPtr klass) { virObjectPtr obj = NULL; @@ -205,7 +211,8 @@ void *virObjectNew(virClassPtr klass) } -void *virObjectLockableNew(virClassPtr klass) +void * +virObjectLockableNew(virClassPtr klass) { virObjectLockablePtr obj; @@ -230,13 +237,15 @@ void *virObjectLockableNew(virClassPtr klass) } -static void virObjectLockableDispose(void *anyobj) +static void +virObjectLockableDispose(void *anyobj) { virObjectLockablePtr obj = anyobj; virMutexDestroy(&obj->lock); } + /** * virObjectUnref: * @anyobj: any instance of virObjectPtr @@ -248,7 +257,8 @@ static void virObjectLockableDispose(void *anyobj) * Returns true if the remaining reference count is * non-zero, false if the object was disposed of */ -bool virObjectUnref(void *anyobj) +bool +virObjectUnref(void *anyobj) { virObjectPtr obj = anyobj; @@ -286,7 +296,8 @@ bool virObjectUnref(void *anyobj) * * Returns @anyobj */ -void *virObjectRef(void *anyobj) +void * +virObjectRef(void *anyobj) { virObjectPtr obj = anyobj; @@ -310,7 +321,8 @@ void *virObjectRef(void *anyobj) * The object must be unlocked before releasing this * reference. */ -void virObjectLock(void *anyobj) +void +virObjectLock(void *anyobj) { virObjectLockablePtr obj = anyobj; @@ -331,7 +343,8 @@ void virObjectLock(void *anyobj) * Release a lock on @anyobj. The lock must have been * acquired by virObjectLock. */ -void virObjectUnlock(void *anyobj) +void +virObjectUnlock(void *anyobj) { virObjectLockablePtr obj = anyobj; @@ -355,8 +368,9 @@ void virObjectUnlock(void *anyobj) * * Returns true if @anyobj is an instance of @klass */ -bool virObjectIsClass(void *anyobj, - virClassPtr klass) +bool +virObjectIsClass(void *anyobj, + virClassPtr klass) { virObjectPtr obj = anyobj; if (!obj) @@ -372,7 +386,8 @@ bool virObjectIsClass(void *anyobj, * * Returns the name of @klass */ -const char *virClassName(virClassPtr klass) +const char * +virClassName(virClassPtr klass) { return klass->name; } @@ -401,7 +416,9 @@ void virObjectFreeCallback(void *opaque) * but with the signature matching the virHashDataFree * typedef. */ -void virObjectFreeHashData(void *opaque, const void *name ATTRIBUTE_UNUSED) +void +virObjectFreeHashData(void *opaque, + const void *name ATTRIBUTE_UNUSED) { virObjectUnref(opaque); } @@ -413,7 +430,8 @@ void virObjectFreeHashData(void *opaque, const void *name ATTRIBUTE_UNUSED) * * Unrefs all members of @list and frees the list itself. */ -void virObjectListFree(void *list) +void +virObjectListFree(void *list) { void **next; @@ -434,7 +452,9 @@ void virObjectListFree(void *list) * * Unrefs all members of @list and frees the list itself. */ -void virObjectListFreeCount(void *list, size_t count) +void +virObjectListFreeCount(void *list, + size_t count) { size_t i; diff --git a/src/util/virobject.h b/src/util/virobject.h index c3ecc1e..f4c292b 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -66,40 +66,61 @@ virClassPtr virClassForObjectLockable(void); # ifndef VIR_PARENT_REQUIRED # define VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(1) # endif -virClassPtr virClassNew(virClassPtr parent, - const char *name, - size_t objectSize, - virObjectDisposeCallback dispose) +virClassPtr +virClassNew(virClassPtr parent, + const char *name, + size_t objectSize, + virObjectDisposeCallback dispose) VIR_PARENT_REQUIRED ATTRIBUTE_NONNULL(2); -const char *virClassName(virClassPtr klass) +const char * +virClassName(virClassPtr klass) ATTRIBUTE_NONNULL(1); -bool virClassIsDerivedFrom(virClassPtr klass, - virClassPtr parent) +bool +virClassIsDerivedFrom(virClassPtr klass, + virClassPtr parent) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -void *virObjectNew(virClassPtr klass) +void * +virObjectNew(virClassPtr klass) ATTRIBUTE_NONNULL(1); -bool virObjectUnref(void *obj); -void *virObjectRef(void *obj); -bool virObjectIsClass(void *obj, - virClassPtr klass) +bool +virObjectUnref(void *obj); + +void * +virObjectRef(void *obj); + +bool +virObjectIsClass(void *obj, + virClassPtr klass) ATTRIBUTE_NONNULL(2); -void virObjectFreeCallback(void *opaque); -void virObjectFreeHashData(void *opaque, const void *name); +void +virObjectFreeCallback(void *opaque); + +void +virObjectFreeHashData(void *opaque, + const void *name); -void *virObjectLockableNew(virClassPtr klass) +void * +virObjectLockableNew(virClassPtr klass) ATTRIBUTE_NONNULL(1); -void virObjectLock(void *lockableobj) +void +virObjectLock(void *lockableobj) ATTRIBUTE_NONNULL(1); -void virObjectUnlock(void *lockableobj) + +void +virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1); -void virObjectListFree(void *list); -void virObjectListFreeCount(void *list, size_t count); +void +virObjectListFree(void *list); + +void +virObjectListFreeCount(void *list, + size_t count); #endif /* __VIR_OBJECT_H */ -- 2.9.4

If virObjectIsClass fails "internally" to virobject.c, create a macro to generate the VIR_WARN describing what the problem is. Also improve the checks and message a bit to indicate which was the failure - whether the obj was NULL or just not the right class Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virobject.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/util/virobject.c b/src/util/virobject.c index 34805d3..9f5f187 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -47,6 +47,16 @@ struct _virClass { virObjectDisposeCallback dispose; }; +#define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass) \ + do { \ + virObjectPtr obj = anyobj; \ + if (!obj) \ + VIR_WARN("Object %p is not a virObject class instance", anyobj);\ + else \ + VIR_WARN("Object %p (%s) is not a %s instance", \ + anyobj, obj->klass->name, #objclass); \ + } while (0) + static virClassPtr virObjectClass; static virClassPtr virObjectLockableClass; @@ -312,14 +322,10 @@ virObjectRef(void *anyobj) static virObjectLockablePtr virObjectGetLockableObj(void *anyobj) { - virObjectPtr obj; - if (virObjectIsClass(anyobj, virObjectLockableClass)) return anyobj; - obj = anyobj; - VIR_WARN("Object %p (%s) is not a virObjectLockable instance", - anyobj, obj ? obj->klass->name : "(unknown)"); + VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass); return NULL; } -- 2.9.4

On Tue, May 30, 2017 at 07:38:16AM -0400, John Ferlan wrote: I'm wondering whether this will risk re-introducing the bug fixed in commit fca4f2334072d87f7faeb2948e6f83201309e1b9 Author: Eric Blake <eblake(a)redhat.com&gt; Date: Thu Dec 12 16:01:15 2013 -0700 object: require maximal alignment in base class I'm also thinking we don't really need to have 2 magic fields in the same struct - we already have a 'magic' field that is initialized from the class object magic value. Now, this existing magic is different for each object subclass - we allocate class magic starting with static unsigned int magicCounter = 0xCAFE0000; I'm thinking though, that we're never going to have > 65556 different sub-classes (well at least not for a long time). So instead of adding this new field you could just check ((object->u.s.magic & 0xCAFE0000) == 0xCAFE0000) Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

On Tue, May 30, 2017 at 10:43:35AM -0400, John Ferlan wrote: Should also put an assert in the virClassNew method that the new magic is <= 0xCAFEFFFF as a safety net for 20 years time when we finally have 65536 classes :-) Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Add a pair of accessor API's: virObjectPoolableHashElementGetPrimaryKey virObjectPoolableHashElementGetSecondaryKey which will return the requested key from the object to the caller. It is up to the caller to check the returned key and error if the return value is NULL. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/libvirt_private.syms | 2 ++ src/util/virobject.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virobject.h | 6 ++++++ 3 files changed, 58 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index eca1980..fea0be7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2271,6 +2271,8 @@ virObjectLock; virObjectLockableNew; virObjectLockableRecursiveNew; virObjectNew; +virObjectPoolableHashElementGetPrimaryKey; +virObjectPoolableHashElementGetSecondaryKey; virObjectPoolableHashElementNew; virObjectRef; virObjectUnlock; diff --git a/src/util/virobject.c b/src/util/virobject.c index 25dbb8f..74299f8 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -441,6 +441,18 @@ virObjectGetLockableObj(void *anyobj) } +static virObjectPoolableHashElementPtr +virObjectGetPoolableHashElementObj(void *anyobj) +{ + if (virObjectIsClass(anyobj, virObjectPoolableHashElementClass)) + return anyobj; + + VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectPoolableHashElementClass); + + return NULL; +} + + /** * virObjectLock: * @anyobj: any instance of virObjectLockablePtr @@ -592,3 +604,41 @@ virObjectListFreeCount(void *list, VIR_FREE(list); } + + +/** + * virObjectPoolableHashElementGetPrimaryKey + * @anyobj: Pointer to a PoolableHashElement object + * + * Returns: Pointer to the primaryKey or NULL on failure + */ +const char * +virObjectPoolableHashElementGetPrimaryKey(void *anyobj) +{ + virObjectPoolableHashElementPtr obj = + virObjectGetPoolableHashElementObj(anyobj); + + if (!obj) + return NULL; + + return obj->primaryKey; +} + + +/** + * virObjectPoolableHashElementGetSecondaryKey + * @anyobj: Pointer to a PoolableHashElement object + * + * Returns: Pointer to the secondaryKey which may be NULL + */ +const char * +virObjectPoolableHashElementGetSecondaryKey(void *anyobj) +{ + virObjectPoolableHashElementPtr obj = + virObjectGetPoolableHashElementObj(anyobj); + + if (!obj) + return NULL; + + return obj->secondaryKey; +} diff --git a/src/util/virobject.h b/src/util/virobject.h index 0377cd5..4706502 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -146,4 +146,10 @@ void virObjectListFreeCount(void *list, size_t count); +const char * +virObjectPoolableHashElementGetPrimaryKey(void *anyobj); + +const char * +virObjectPoolableHashElementGetSecondaryKey(void *anyobj); + #endif /* __VIR_OBJECT_H */ -- 2.9.4

Add new API's for object management: virObjectPoolableDefGetDef - Just return the obj->def from the object. virObjectPoolableDefSetDef - If the new @def is non-NULL, replace the previous obj->def with the new @def argument calling the obj->defFreeFunc on the previous obj->def. If the @def is NULL, then just clear the obj->def leaving the caller to handle removal of the previous obj->def. virObjectPoolableDefGetNewDef - Just return the obj->newDef from the object. virObjectPoolableDefSetNewDef - If the new @newDef is non-NULL, replace the previous obj->newDef with the new @newDef argument calling the obj->defFreeFunc on the previous obj->newDef. If the @newDef is NULL, then just clear the obj->newDef leaving the caller to handle removal of the previous obj->newDef. virObjectPoolableDefSwapNewDef - Manage swapping the obj->newDef into obj->def by first calling the obj->defFreeFunc on the current obj->def and then stealing the obj->newDef into obj->def. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/libvirt_private.syms | 5 ++ src/util/virobject.c | 143 +++++++++++++++++++++++++++++++++++++++++++++++ src/util/virobject.h | 17 ++++++ 3 files changed, 165 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4fad7c8..f6e40c7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2272,7 +2272,12 @@ virObjectLock; virObjectLockableNew; virObjectLockableRecursiveNew; virObjectNew; +virObjectPoolableDefGetDef; +virObjectPoolableDefGetNewDef; virObjectPoolableDefNew; +virObjectPoolableDefSetDef; +virObjectPoolableDefSetNewDef; +virObjectPoolableDefStealNewDef; virObjectPoolableHashElementGetPrimaryKey; virObjectPoolableHashElementGetSecondaryKey; virObjectPoolableHashElementNew; diff --git a/src/util/virobject.c b/src/util/virobject.c index d1fc3f0..ef253b6 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -536,6 +536,18 @@ virObjectGetPoolableHashElementObj(void *anyobj) } +static virObjectPoolableDefPtr +virObjectGetPoolableDefObj(void *anyobj) +{ + if (virObjectIsClass(anyobj, virObjectPoolableDefClass)) + return anyobj; + + VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectPoolableDefClass); + + return NULL; +} + + /** * virObjectLock: * @anyobj: any instance of virObjectLockablePtr @@ -725,3 +737,134 @@ virObjectPoolableHashElementGetSecondaryKey(void *anyobj) return obj->secondaryKey; } + + +/** + * virObjectPoolableDefGetDef + * @anyobj: Pointer to a locked PoolableDef object + * + * Returns: Pointer to the object @def or NULL on failure + */ +void * +virObjectPoolableDefGetDef(void *anyobj) +{ + virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj); + + if (!obj) + return NULL; + + return obj->def; +} + + +/** + * virObjectPoolableDefSetDef + * @anyobj: Pointer to a locked PoolableDef object + * @def: Opaque object definition to replace in the object + * + * Set the @def of the object - this may be NULL if clearing the @def + * from the object in which case it is up to the caller to handle managing + * the previous obj->def. If @def is not NULL, this includes a call to the + * defFreeFunc prior to the set. + * + * Returns 0 on success, -1 on failure + */ +int +virObjectPoolableDefSetDef(void *anyobj, + void *def) +{ + virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj); + + if (!obj) + return -1; + + if (def) { + obj->defFreeFunc(obj->def); + obj->def = def; + } else { + obj->def = NULL; + } + + return 0; +} + + +/** + * virObjectPoolableDefGetNewDef + * @anyobj: Pointer to a locked PoolableDef object + * + * Returns: Pointer to the object 'newDef' or NULL on failure + */ +void * +virObjectPoolableDefGetNewDef(void *anyobj) +{ + virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj); + + if (!obj) + return NULL; + + return obj->newDef; +} + + +/** + * virObjectPoolableDefSetNewDef + * @anyobj: Pointer to a locked PoolableDef object + * @newDef: Opaque 'newDef' to replace in the object, may be NULL + * + * Set the 'newDef' of the object - this may be NULL if clearing the newDef + * from the object in which case it is up to the caller to handle managing + * the previous newDef. If @newDef is not NULL, this includes a call to the + * defFreeFunc prior to the set. + * + * Returns 0 on success, -1 on failure + */ +int +virObjectPoolableDefSetNewDef(void *anyobj, + void *newDef) +{ + virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj); + + if (!obj) + return -1; + + if (newDef) { + obj->defFreeFunc(obj->newDef); + obj->newDef = newDef; + } else { + obj->newDef = NULL; + } + + return 0; +} + + +/** + * virObjectPoolableDefSwapNewDef + * @anyobj: Pointer to a locked PoolableDef object + * + * Manage swapping the obj->newDef into obj->def in one pass. + * When called, the previous obj->def will be free'd and the + * existing defDef will take it's place. + * + * Returns 0 on success, -1 on failure + */ +int +virObjectPoolableDefStealNewDef(void *anyobj) +{ + virObjectPoolableDefPtr obj = virObjectGetPoolableDefObj(anyobj); + + if (!obj) + return -1; + + if (!obj->newDef) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to steal NULL newDef")); + return -1; + } + + obj->defFreeFunc(obj->def); + VIR_STEAL_PTR(obj->def, obj->newDef); + + return 0; +} diff --git a/src/util/virobject.h b/src/util/virobject.h index 9bbbece..d3aeb73 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -177,4 +177,21 @@ virObjectPoolableHashElementGetPrimaryKey(void *anyobj); const char * virObjectPoolableHashElementGetSecondaryKey(void *anyobj); +void * +virObjectPoolableDefGetDef(void *anyobj); + +int +virObjectPoolableDefSetDef(void *anyobj, + void *def); + +void * +virObjectPoolableDefGetNewDef(void *anyobj); + +int +virObjectPoolableDefSetNewDef(void *anyobj, + void *newDef); + +int +virObjectPoolableDefStealNewDef(void *anyobj); + #endif /* __VIR_OBJECT_H */ -- 2.9.4