Since the class it represents is based on virObjectRWLockableClass and in order to make sure we diffentiate lest anyone somehow believes they could use virObjectLockRead for a virObjectLockableClass, let's rename the API to use the RW in the name. Besides the RW locks refer to pthread_rwlock_{init|rdlock|wrlock|unlock|destroy} while the other locks refer to pthread_mutex_{init|lock|unlock|destroy}. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virdomainobjlist.c | 18 +++++++++--------- src/libvirt_private.syms | 2 +- src/util/virobject.c | 11 ++++++++--- src/util/virobject.h | 2 +- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 1d027a4..9bc6603 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -118,7 +118,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms, bool ref) { virDomainObjPtr obj; - virObjectLockRead(doms); + virObjectRWLockRead(doms); obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL); if (ref) { virObjectRef(obj); @@ -160,7 +160,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObjPtr obj; - virObjectLockRead(doms); + virObjectRWLockRead(doms); virUUIDFormat(uuid, uuidstr); obj = virHashLookup(doms->objs, uuidstr); @@ -204,7 +204,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, { virDomainObjPtr obj; - virObjectLockRead(doms); + virObjectRWLockRead(doms); obj = virHashLookup(doms->objsName, name); virObjectRef(obj); virObjectUnlock(doms); @@ -653,7 +653,7 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms, virConnectPtr conn) { struct virDomainObjListData data = { filter, conn, active, 0 }; - virObjectLockRead(doms); + virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCount, &data); virObjectUnlock(doms); return data.count; @@ -697,7 +697,7 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms, { struct virDomainIDData data = { filter, conn, 0, maxids, ids }; - virObjectLockRead(doms); + virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data); virObjectUnlock(doms); return data.numids; @@ -751,7 +751,7 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms, struct virDomainNameData data = { filter, conn, 0, 0, maxnames, names }; size_t i; - virObjectLockRead(doms); + virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data); virObjectUnlock(doms); if (data.oom) { @@ -792,7 +792,7 @@ virDomainObjListForEach(virDomainObjListPtr doms, struct virDomainListIterData data = { callback, opaque, 0, }; - virObjectLockRead(doms); + virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListHelper, &data); virObjectUnlock(doms); return data.ret; @@ -925,7 +925,7 @@ virDomainObjListCollect(virDomainObjListPtr domlist, { struct virDomainListData data = { NULL, 0 }; - virObjectLockRead(domlist); + virObjectRWLockRead(domlist); sa_assert(domlist->objs); if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) { virObjectUnlock(domlist); @@ -962,7 +962,7 @@ virDomainObjListConvert(virDomainObjListPtr domlist, *nvms = 0; *vms = NULL; - virObjectLockRead(domlist); + virObjectRWLockRead(domlist); for (i = 0; i < ndoms; i++) { virDomainPtr dom = doms[i]; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 37b815c..99302d2 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2305,10 +2305,10 @@ virObjectListFree; virObjectListFreeCount; virObjectLock; virObjectLockableNew; -virObjectLockRead; virObjectNew; virObjectRef; virObjectRWLockableNew; +virObjectRWLockRead; virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index b1bb378..b97f251 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -399,7 +399,7 @@ virObjectLock(void *anyobj) /** - * virObjectLockRead: + * virObjectRWLockRead: * @anyobj: any instance of virObjectRWLockable * * Acquire a read lock on @anyobj. The lock must be @@ -409,9 +409,14 @@ virObjectLock(void *anyobj) * on the object before locking it (eg virObjectRef). * The object must be unlocked before releasing this * reference. + * + * NB: It's possible to return without the lock if + * @anyobj was invalid - this has been considered + * a programming error rather than something that + * should be checked. */ void -virObjectLockRead(void *anyobj) +virObjectRWLockRead(void *anyobj) { if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { virObjectRWLockablePtr obj = anyobj; @@ -429,7 +434,7 @@ virObjectLockRead(void *anyobj) * @anyobj: any instance of virObjectLockable or virObjectRWLockable * * Release a lock on @anyobj. The lock must have been acquired by - * virObjectLock or virObjectLockRead. + * virObjectLock or virObjectRWLockRead. */ void virObjectUnlock(void *anyobj) diff --git a/src/util/virobject.h b/src/util/virobject.h index 5985fad..e7ca983 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -125,7 +125,7 @@ virObjectLock(void *lockableobj) ATTRIBUTE_NONNULL(1); void -virObjectLockRead(void *lockableobj) +virObjectRWLockRead(void *lockableobj) ATTRIBUTE_NONNULL(1); void -- 2.9.4

On 08/01/2017 09:24 AM, Michal Privoznik wrote: Just differentiate from my dictionary. 'lest' is someone colloquial - I can alter it though. Well function naming guidelines weren't in place when virObjectLock (and Unlock) were added, but they are now. I read the response and the others... I'm torn between RWLockRead and just LockRead. I really don't care either way. I went with RW for the stated reason though - fear that someone like you sees virObjectLockRead (or worse virObjectLockWrite) and believes that is what they want. If they are not operating on an RWLockableObject, then they really don't get the lock and because we've decided to not error out in that case, they don't have the safety they thought they had. Maybe I'm wrong, but I have to present that argument. As for altering virObjectLock to virObjectLockableLock - that ship sailed long ago. I would say it would be virObjectMutexLock though to be more precise, but using virObjectLock as a shorthand since there was only one locking subsystem available is understandable. Time has brought forth some new options and now we have to adjust the new code to fit the more recent models. The old code could be adjusted, but there's far too many places that need change and no one wants that insanely impossible task. I suppose I could also see just reason to go with virObjectLockRWRdLock and virObjectLockRWWrLock (and virObjectUnlockRW). I haven't trained my editor to choose API names for me. Not sure there's a perfect solution for this - perhaps multiple opinions though. I suppose all that really matters is we decide either: 1. Leave things as they are - completely 2. Alter the naming scheme in some way I can live with #1 even though I'm concerned about mis-use. Also, I thought using overloaded functions was something that long ago was decided to be less desirable. That is the Lock and Unlock operate on two different object types based on something stored in the object instead of two separate API's. The call is to two very different lower level API's as well that cannot be used interchangeably. While IIUC the goal would be to some day change all virObjectLock()'s to be either LockRead or LockWrite and do away with virObjectLock and any sort of reference to virMutexLock's and that's a noble goal. However, I would also think it could be awhile before that's realizable. So yes, it's a conundrum and I can be talked into dropping this series. Although I do still see value in the "magic number check" prior to using a non NULL @obj (a/k/a @anyobj). John FWIW: As it relates to common object - since I've chosen to use a LockableLock and RWLockable as "parent"'s to my new object children, those conditions that check virObjectIsClass(anyobj,... get a bit more ugly looking or are replaced by something that can check the parent or the parent's parent (letting someone else figure out 3 levels of a family tree).

Instead of making virObjectLock be the entry point for two different types of locks, let's create a virObjectRWLockWrite API which will only handle the virObjectRWLockableClass objects. Use the new virObjectRWLockWrite for the virdomainobjlist code in order to handle the Add, Remove, Rename, and Load operations that need to be very synchronous. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virdomainobjlist.c | 8 ++++---- src/libvirt_private.syms | 1 + src/util/virobject.c | 33 ++++++++++++++++++++++++++++++++- src/util/virobject.h | 4 ++++ 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 9bc6603..573032f 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -327,7 +327,7 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, { virDomainObjPtr ret; - virObjectLock(doms); + virObjectRWLockWrite(doms); ret = virDomainObjListAddLocked(doms, def, xmlopt, flags, oldDef); virObjectUnlock(doms); return ret; @@ -349,7 +349,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms, virObjectRef(dom); virObjectUnlock(dom); - virObjectLock(doms); + virObjectRWLockWrite(doms); virObjectLock(dom); virHashRemoveEntry(doms->objs, uuidstr); virHashRemoveEntry(doms->objsName, dom->def->name); @@ -394,7 +394,7 @@ virDomainObjListRename(virDomainObjListPtr doms, * hold a lock on dom but not refcount it. */ virObjectRef(dom); virObjectUnlock(dom); - virObjectLock(doms); + virObjectRWLockWrite(doms); virObjectLock(dom); virObjectUnref(dom); @@ -573,7 +573,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, if ((rc = virDirOpenIfExists(&dir, configDir)) <= 0) return rc; - virObjectLock(doms); + virObjectRWLockWrite(doms); while ((ret = virDirRead(dir, &entry, configDir)) > 0) { virDomainObjPtr dom; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 99302d2..7f2f6d6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2309,6 +2309,7 @@ virObjectNew; virObjectRef; virObjectRWLockableNew; virObjectRWLockRead; +virObjectRWLockWrite; virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index b97f251..f49af62 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -430,11 +430,42 @@ virObjectRWLockRead(void *anyobj) /** + * virObjectRWLockWrite: + * @anyobj: any instance of virObjectRWLockable + * + * Acquire a write lock on @anyobj. The lock must be + * released by virObjectUnlock. + * + * The caller is expected to have acquired a reference + * on the object before locking it (eg virObjectRef). + * The object must be unlocked before releasing this + * reference. + * + * NB: It's possible to return without the lock if + * @anyobj was invalid - this has been considered + * a programming error rather than something that + * should be checked. + */ +void +virObjectRWLockWrite(void *anyobj) +{ + if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { + virObjectRWLockablePtr obj = anyobj; + virRWLockWrite(&obj->lock); + } else { + virObjectPtr obj = anyobj; + VIR_WARN("Object %p (%s) is not a virObjectRWLockable instance", + anyobj, obj ? obj->klass->name : "(unknown)"); + } +} + + +/** * virObjectUnlock: * @anyobj: any instance of virObjectLockable or virObjectRWLockable * * Release a lock on @anyobj. The lock must have been acquired by - * virObjectLock or virObjectRWLockRead. + * virObjectLock, virObjectRWLockRead, or virObjectRWLockWrite. */ void virObjectUnlock(void *anyobj) diff --git a/src/util/virobject.h b/src/util/virobject.h index e7ca983..24ee6dd 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -129,6 +129,10 @@ virObjectRWLockRead(void *lockableobj) ATTRIBUTE_NONNULL(1); void +virObjectRWLockWrite(void *lockableobj) + ATTRIBUTE_NONNULL(1); + +void virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1); -- 2.9.4

On 08/01/2017 02:05 AM, John Ferlan wrote: If we're really worried about this (and don't want to abort()), we might consider raising this to VIR_ERROR. I'm a programmer, I don't care about warnings O:-) Michal

Rather than overload virObjectUnlock as commit id '77f4593b' has done, create a separate virObjectRWUnlock API that will force the consumers to make the proper decision regarding unlocking the RWLock's. Similar to the RWLockRead and RWLockWrite, use the virObjectGetRWLockableObj helper. This restores the virObjectUnlock code to using the virObjectGetLockableObj. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/conf/virdomainobjlist.c | 36 ++++++++++++++++++------------------ src/libvirt_private.syms | 1 + src/util/virobject.c | 45 +++++++++++++++++++++++++++++---------------- src/util/virobject.h | 4 ++++ 4 files changed, 52 insertions(+), 34 deletions(-) diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 573032f..d874133 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -122,7 +122,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms, obj = virHashSearch(doms->objs, virDomainObjListSearchID, &id, NULL); if (ref) { virObjectRef(obj); - virObjectUnlock(doms); + virObjectRWUnlock(doms); } if (obj) { virObjectLock(obj); @@ -134,7 +134,7 @@ virDomainObjListFindByIDInternal(virDomainObjListPtr doms, } } if (!ref) - virObjectUnlock(doms); + virObjectRWUnlock(doms); return obj; } @@ -166,7 +166,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, obj = virHashLookup(doms->objs, uuidstr); if (ref) { virObjectRef(obj); - virObjectUnlock(doms); + virObjectRWUnlock(doms); } if (obj) { virObjectLock(obj); @@ -178,7 +178,7 @@ virDomainObjListFindByUUIDInternal(virDomainObjListPtr doms, } } if (!ref) - virObjectUnlock(doms); + virObjectRWUnlock(doms); return obj; } @@ -207,7 +207,7 @@ virDomainObjPtr virDomainObjListFindByName(virDomainObjListPtr doms, virObjectRWLockRead(doms); obj = virHashLookup(doms->objsName, name); virObjectRef(obj); - virObjectUnlock(doms); + virObjectRWUnlock(doms); if (obj) { virObjectLock(obj); if (obj->removing) { @@ -329,7 +329,7 @@ virDomainObjPtr virDomainObjListAdd(virDomainObjListPtr doms, virObjectRWLockWrite(doms); ret = virDomainObjListAddLocked(doms, def, xmlopt, flags, oldDef); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return ret; } @@ -355,7 +355,7 @@ void virDomainObjListRemove(virDomainObjListPtr doms, virHashRemoveEntry(doms->objsName, dom->def->name); virObjectUnlock(dom); virObjectUnref(dom); - virObjectUnlock(doms); + virObjectRWUnlock(doms); } @@ -420,7 +420,7 @@ virDomainObjListRename(virDomainObjListPtr doms, ret = 0; cleanup: - virObjectUnlock(doms); + virObjectRWUnlock(doms); VIR_FREE(old_name); return ret; } @@ -609,7 +609,7 @@ virDomainObjListLoadAllConfigs(virDomainObjListPtr doms, } VIR_DIR_CLOSE(dir); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return ret; } @@ -655,7 +655,7 @@ virDomainObjListNumOfDomains(virDomainObjListPtr doms, struct virDomainObjListData data = { filter, conn, active, 0 }; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCount, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return data.count; } @@ -699,7 +699,7 @@ virDomainObjListGetActiveIDs(virDomainObjListPtr doms, 0, maxids, ids }; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCopyActiveIDs, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return data.numids; } @@ -753,7 +753,7 @@ virDomainObjListGetInactiveNames(virDomainObjListPtr doms, size_t i; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListCopyInactiveNames, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); if (data.oom) { for (i = 0; i < data.numnames; i++) VIR_FREE(data.names[i]); @@ -794,7 +794,7 @@ virDomainObjListForEach(virDomainObjListPtr doms, }; virObjectRWLockRead(doms); virHashForEach(doms->objs, virDomainObjListHelper, &data); - virObjectUnlock(doms); + virObjectRWUnlock(doms); return data.ret; } @@ -928,12 +928,12 @@ virDomainObjListCollect(virDomainObjListPtr domlist, virObjectRWLockRead(domlist); sa_assert(domlist->objs); if (VIR_ALLOC_N(data.vms, virHashSize(domlist->objs)) < 0) { - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); return -1; } virHashForEach(domlist->objs, virDomainObjListCollectIterator, &data); - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); virDomainObjListFilter(&data.vms, &data.nvms, conn, filter, flags); @@ -972,7 +972,7 @@ virDomainObjListConvert(virDomainObjListPtr domlist, if (skip_missing) continue; - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); virReportError(VIR_ERR_NO_DOMAIN, _("no domain with matching uuid '%s' (%s)"), uuidstr, dom->name); @@ -982,12 +982,12 @@ virDomainObjListConvert(virDomainObjListPtr domlist, virObjectRef(vm); if (VIR_APPEND_ELEMENT(*vms, *nvms, vm) < 0) { - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); virObjectUnref(vm); goto error; } } - virObjectUnlock(domlist); + virObjectRWUnlock(domlist); sa_assert(*vms); virDomainObjListFilter(vms, nvms, conn, filter, flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7f2f6d6..97aaf37 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2310,6 +2310,7 @@ virObjectRef; virObjectRWLockableNew; virObjectRWLockRead; virObjectRWLockWrite; +virObjectRWUnlock; virObjectUnlock; virObjectUnref; diff --git a/src/util/virobject.c b/src/util/virobject.c index c1e4474..85e5a53 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -428,7 +428,7 @@ virObjectLock(void *anyobj) * @anyobj: any instance of virObjectRWLockable * * Acquire a read lock on @anyobj. The lock must be - * released by virObjectUnlock. + * released by virObjectRWUnlock. * * The caller is expected to have acquired a reference * on the object before locking it (eg virObjectRef). @@ -457,7 +457,7 @@ virObjectRWLockRead(void *anyobj) * @anyobj: any instance of virObjectRWLockable * * Acquire a write lock on @anyobj. The lock must be - * released by virObjectUnlock. + * released by virObjectRWUnlock. * * The caller is expected to have acquired a reference * on the object before locking it (eg virObjectRef). @@ -483,26 +483,39 @@ virObjectRWLockWrite(void *anyobj) /** * virObjectUnlock: - * @anyobj: any instance of virObjectLockable or virObjectRWLockable + * @anyobj: any instance of virObjectLockable * * Release a lock on @anyobj. The lock must have been acquired by - * virObjectLock, virObjectRWLockRead, or virObjectRWLockWrite. + * virObjectLock. */ void virObjectUnlock(void *anyobj) { - if (virObjectIsClass(anyobj, virObjectLockableClass)) { - virObjectLockablePtr obj = anyobj; - virMutexUnlock(&obj->lock); - } else if (virObjectIsClass(anyobj, virObjectRWLockableClass)) { - virObjectRWLockablePtr obj = anyobj; - virRWLockUnlock(&obj->lock); - } else { - virObjectPtr obj = anyobj; - VIR_WARN("Object %p (%s) is not a virObjectLockable " - "nor virObjectRWLockable instance", - anyobj, obj ? obj->klass->name : "(unknown)"); - } + virObjectLockablePtr obj = virObjectGetLockableObj(anyobj); + + if (!obj) + return; + + virMutexUnlock(&obj->lock); +} + + +/** + * virObjectRWUnlock: + * @anyobj: any instance of virObjectRWLockable + * + * Release a lock on @anyobj. The lock must have been acquired by + * virObjectRWLockRead or virObjectRWLockWrite. + */ +void +virObjectRWUnlock(void *anyobj) +{ + virObjectRWLockablePtr obj = virObjectGetRWLockableObj(anyobj); + + if (!obj) + return; + + virRWLockUnlock(&obj->lock); } diff --git a/src/util/virobject.h b/src/util/virobject.h index 24ee6dd..ac6cf22 100644 --- a/src/util/virobject.h +++ b/src/util/virobject.h @@ -137,6 +137,10 @@ virObjectUnlock(void *lockableobj) ATTRIBUTE_NONNULL(1); void +virObjectRWUnlock(void *lockableobj) + ATTRIBUTE_NONNULL(1); + +void virObjectListFree(void *list); void -- 2.9.4

The virObjectIsClass API has only ever checked object validity based on if the @obj is not NULL and it was derived from some class. While this has worked well in general, there is one additional check that could be made prior to calling virClassIsDerivedFrom which loops through the classes checking the magic number against the klass expected magic number. If by chance a non virObject is passed, rather than assuming the void * @obj is a _virObject and thus offsetting to obj->klass, obj->magic, and obj->parent, let's check that the void * @obj has at least the "base part" of the magic number in the right place and generate a more specific VIR_WARN message if not. There are many consumers to virObjectIsClass, include the locking primitives virObject{Lock|Unlock}, virObjectRWLock{Read|Write}, and virObjectRWUnlock. For those callers, the locking call will not fail, but it also will not attempt a virMutex* call which will "most likely" fail since the &obj->lock is used. In order to avoid some possible future wrap on the 0xCAFExxxx value, add a check during initialization that some new class won't cause the wrap. Should be good for a few years at least! It is still left up to the caller to handle the failed API calls just as it would be if it passed a NULL opaque pointer anyobj. Signed-off-by: John Ferlan <jferlan(a)redhat.com&gt; --- src/util/virobject.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/util/virobject.c b/src/util/virobject.c index af3f252..54d78b0 100644 --- a/src/util/virobject.c +++ b/src/util/virobject.c @@ -47,14 +47,21 @@ struct _virClass { virObjectDisposeCallback dispose; }; +#define VIR_OBJECT_NOTVALID(obj) (!obj || ((obj->u.s.magic & 0xFFFF0000) != 0xCAFE0000)) + #define VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, objclass) \ do { \ virObjectPtr obj = anyobj; \ - if (!obj) \ - VIR_WARN("Object cannot be NULL"); \ - else \ + if (VIR_OBJECT_NOTVALID(obj)) { \ + if (!obj) \ + VIR_WARN("Object cannot be NULL"); \ + else \ + VIR_WARN("Object %p has a bad magic number %X", \ + obj, obj->u.s.magic); \ + } else { \ VIR_WARN("Object %p (%s) is not a %s instance", \ anyobj, obj->klass->name, #objclass); \ + } \ } while (0) @@ -177,9 +184,14 @@ virClassNew(virClassPtr parent, goto error; klass->parent = parent; + klass->magic = virAtomicIntInc(&magicCounter); + if (klass->magic > 0xCAFEFFFF) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("too many object classes defined")); + goto error; + } if (VIR_STRDUP(klass->name, name) < 0) goto error; - klass->magic = virAtomicIntInc(&magicCounter); klass->objectSize = objectSize; klass->dispose = dispose; @@ -535,7 +547,7 @@ virObjectIsClass(void *anyobj, virClassPtr klass) { virObjectPtr obj = anyobj; - if (!obj) + if (VIR_OBJECT_NOTVALID(obj)) return false; return virClassIsDerivedFrom(obj->klass, klass); -- 2.9.4