Libguestfs - Jul 2019 - [libnbd PATCH] lib: Call read/extent(FREE) before callback(VALID|FREE)

Some callers of nbd_aio_pread_structured_callback or
nbd_aio_block_status_callback share the same opaque pointer to both
calls, with the intent to only free the pointer on the completion
callback.  Although our documentation does not explicitly specify the
order in which callbacks are made, allowing the callback(FREE) to
occur prior to the read(FREE) puts a burden on the client to not
dereference the user_data in the read() function if the VALID bit is
not set.  If the client follows our suggested practice of an immediate
return 0 if the VAILD bit is not set, then this is the case, but we
can't guarantee all clients will use that pattern.  And, it's fairly
easy to guarantee that ALL of our read/extent callbacks are made prior
to any of our callback() use, by utilizing the FINISH state of
structured replies to catch the few cases where we are unable to send
VALID|FREE.

Note that nbd_internal_retire_and_free_command still has to check
whether we have freed the callbacks, as nbd_close() can strand
commands before the FINISH state.
---
 generator/states-reply-structured.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/generator/states-reply-structured.c
b/generator/states-reply-structured.c
index 20b0e9e..ff5b727 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -540,11 +540,20 @@ valid_flags (struct nbd_handle *h)
   return 0;

  REPLY.STRUCTURED_REPLY.FINISH:
+  struct command *cmd = h->reply_cmd;
   uint16_t flags;

   flags = be16toh (h->sbuf.sr.structured_reply.flags);
-  if (flags & NBD_REPLY_FLAG_DONE)
+  if (flags & NBD_REPLY_FLAG_DONE) {
+    if (cmd->type == NBD_CMD_BLOCK_STATUS && cmd->cb.fn.extent)
+      cmd->cb.fn.extent (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data,
+                         NULL, 0, NULL, 0, NULL);
+    if (cmd->type == NBD_CMD_READ && cmd->cb.fn.read)
+      cmd->cb.fn.read (LIBNBD_CALLBACK_FREE, cmd->cb.fn_user_data,
+                       NULL, 0, 0, 0, NULL);
+    cmd->cb.fn.read = NULL;
     SET_NEXT_STATE (%^FINISH_COMMAND);
+  }
   else {
     h->reply_cmd = NULL;
     SET_NEXT_STATE (%.READY);
-- 
2.20.1