On 03/15/2013 12:28 AM, Eric Blake wrote:
- if (conn->closeFreeCallback)
+ if (conn->closeCallback)
+ conn->closeCallback = NULL;
The if is pointless. Just blindly set conn->closeCallback to NULL.
agreed
+
+ if (conn->closeFreeCallback) {
conn->closeFreeCallback(conn->closeOpaque);
+ conn->closeFreeCallback = NULL;
+ conn->closeOpaque = NULL;
Clearing conn->closeOpaque is pointless; it is only ever used depending
on conn->closeFreeCallback, and leaving it non-NULL doesn't hurt.
I know, and didn't do it initially, but then wanted to make it common
with the callback deregistering code. And a small portion of paranoia
doesn't hurt as I have come to learn.
...Wouldn't it be better to stash a copy of the callback pointer while
the mutex is held, but avoid calling the callback until after the mutex
is unlocked? Something like:
<TYPE> cb = NULL;
void* opaque;
virMutexLock(&conn->lock);
conn->closeDispatch = false;
if (conn->closeUnregisterCount != closeUnregisterCount) {
cb = closeFreeCallback;
opaque = closeOpaque;
}
virMutexUnlock(&conn->lock);
if (cb)
cb(opaque);
maybe, but this is again common to the other places where the
freeing callback is invoked, i.e. within the lock.
Waiting for Dan's comments...
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martina Köderitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list