Re: [Libguestfs] [PATCH libnbd v2 04/10] lib: Permit .callback = NULL, .free != NULL.
On 8/15/19 4:56 AM, Richard W.M. Jones wrote:
Previously the .free function of a callback was not called if the
.callback field was NULL, because the callback as a whole would be
considered to be "null".
This change allows you to register callbacks where the .callback field
is NULL, but the .free field is != NULL, meaning that the callback is
freed after the last time it would have been used.
This is mainly convenient for language bindings where we sometimes
want to register a free function to clean up a persistent buffer, but
we don't need the associated completion callback to be actually
called.
---
docs/libnbd.pod | 15 +++++++++++++++
lib/internal.h | 18 +++++++++---------
2 files changed, 24 insertions(+), 9 deletions(-)
+++ b/lib/internal.h
@@ -274,20 +274,20 @@ struct command {
};
/* Test if a callback is "null" or not, and set it to null. */
-#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL)
+#define CALLBACK_IS_NULL(cb) ((cb).callback == NULL && (cb).free == NULL)
Semantic change. In generator, you used CALLBACK_IS_NULL() for both
Closure and OClosure. For OClosure, the new semantics are still
correct. But for Closure, we now no longer return EFAULT when the
callback itself is missing but a .free was provided. This changes
pread_structured and block_status to accept NULL for the callback; which
is probably not a good idea.
In short, there are some points in the code that care only whether
.callback is NULL, and others that care whether both pointers are NULL.
#define CALLBACK_IS_NOT_NULL(cb) (! CALLBACK_IS_NULL ((cb)))
-#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL)
+#define SET_CALLBACK_TO_NULL(cb) ((cb).callback = NULL, (cb).free = NULL)
/* Call a callback. */
-#define CALL_CALLBACK(cb, ...) \
- (cb).callback ((cb).user_data, ##__VA_ARGS__)
+#define CALL_CALLBACK(cb, ...) \
+ ((cb).callback != NULL ? (cb).callback ((cb).user_data, ##__VA_ARGS__) : 0)
This one is nice, though.
/* Free a callback. */
-#define FREE_CALLBACK(cb) \
- do { \
- if (CALLBACK_IS_NOT_NULL (cb) && (cb).free != NULL) \
- (cb).free ((cb).user_data); \
- SET_CALLBACK_TO_NULL (cb); \
+#define FREE_CALLBACK(cb) \
+ do { \
+ if ((cb).free != NULL) \
+ (cb).free ((cb).user_data); \
+ SET_CALLBACK_TO_NULL (cb); \
The SET_CALLBACK_TO_NULL usage is mostly dead code during command
retirement (as we are about to free() the struct containing cb in the
first place), but still vital when clearing the debug callback. So this
part is also fine.
} while (0)
/* aio.c */
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 488 bytes)