[Libguestfs] [PATCH libnbd v2 01/10] generator: Remove cases where we call the free callback early.

[Richard W.M. Jones]

Earlier on we had the ‘valid_flag’.  It was possible to set this to
VALID|FREE avoiding an additional function call.  Since we got rid of
this flag in favour of two separate functions there is no possible way
to avoid an extra function call, and freeing the callbacks close to
the point of last use is therefore no longer worthwhile.  Instead free
all command callbacks in one place
(lib/aio.c:nbd_internal_retire_and_free_command).

Thanks: Eric Blake for pointing this out.
---
 generator/states-reply-simple.c     |  1 -
 generator/states-reply-structured.c | 17 -----------------
 generator/states-reply.c            |  1 -
 generator/states.c                  |  1 -
 4 files changed, 20 deletions(-)

diff --git a/generator/states-reply-simple.c b/generator/states-reply-simple.c
index 8e3d7f1..2f83e6f 100644
--- a/generator/states-reply-simple.c
+++ b/generator/states-reply-simple.c
@@ -69,7 +69,6 @@
                          cmd->offset, LIBNBD_READ_DATA,
                          &error) == -1)
         cmd->error = error ? error : EPROTO;
-      FREE_CALLBACK (cmd->cb.fn.chunk);
     }
 
     SET_NEXT_STATE (%^FINISH_COMMAND);
diff --git a/generator/states-reply-structured.c b/generator/states-reply-structured.c
index 2e327ce..a1641d4 100644
--- a/generator/states-reply-structured.c
+++ b/generator/states-reply-structured.c
@@ -295,7 +295,6 @@
       }
       if (cmd->type == NBD_CMD_READ && cmd->cb.fn.chunk.callback) {
         int scratch = error;
-        uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags);
 
         /* Different from successful reads: inform the callback about the
          * current error rather than any earlier one. If the callback fails
@@ -307,8 +306,6 @@
                            &scratch) == -1)
           if (cmd->error == 0)
             cmd->error = scratch;
-        if (flags & NBD_REPLY_FLAG_DONE)
-          FREE_CALLBACK (cmd->cb.fn.chunk);
       }
     }
 
@@ -390,15 +387,12 @@
     assert (cmd); /* guaranteed by CHECK */
     if (cmd->cb.fn.chunk.callback) {
       int error = cmd->error;
-      uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags);
 
       if (CALL_CALLBACK (cmd->cb.fn.chunk, cmd->data + (offset - cmd->offset),
                          length - sizeof offset, offset,
                          LIBNBD_READ_DATA, &error) == -1)
         if (cmd->error == 0)
           cmd->error = error ? error : EPROTO;
-      if (flags & NBD_REPLY_FLAG_DONE)
-        FREE_CALLBACK (cmd->cb.fn.chunk);
     }
 
     SET_NEXT_STATE (%FINISH);
@@ -454,7 +448,6 @@
     memset (cmd->data + offset, 0, length);
     if (cmd->cb.fn.chunk.callback) {
       int error = cmd->error;
-      uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags);
 
       if (CALL_CALLBACK (cmd->cb.fn.chunk,
                          cmd->data + offset, length,
@@ -462,8 +455,6 @@
                          LIBNBD_READ_HOLE, &error) == -1)
         if (cmd->error == 0)
           cmd->error = error ? error : EPROTO;
-      if (flags & NBD_REPLY_FLAG_DONE)
-        FREE_CALLBACK (cmd->cb.fn.chunk);
     }
 
     SET_NEXT_STATE(%FINISH);
@@ -509,7 +500,6 @@
     if (meta_context) {
       /* Call the caller's extent function. */
       int error = cmd->error;
-      uint16_t flags = be16toh (h->sbuf.sr.structured_reply.flags);
 
       if (CALL_CALLBACK (cmd->cb.fn.extent,
                          meta_context->name, cmd->offset,
@@ -517,8 +507,6 @@
                          &error) == -1)
         if (cmd->error == 0)
           cmd->error = error ? error : EPROTO;
-      if (flags & NBD_REPLY_FLAG_DONE)
-        FREE_CALLBACK (cmd->cb.fn.extent);
     }
     else
       /* Emit a debug message, but ignore it. */
@@ -530,15 +518,10 @@
   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 (cmd->type == NBD_CMD_BLOCK_STATUS)
-      FREE_CALLBACK (cmd->cb.fn.extent);
-    if (cmd->type == NBD_CMD_READ)
-      FREE_CALLBACK (cmd->cb.fn.chunk);
     SET_NEXT_STATE (%^FINISH_COMMAND);
   }
   else {
diff --git a/generator/states-reply.c b/generator/states-reply.c
index 41dcf8d..575a6d1 100644
--- a/generator/states-reply.c
+++ b/generator/states-reply.c
@@ -174,7 +174,6 @@ save_reply_state (struct nbd_handle *h)
 
     assert (cmd->type != NBD_CMD_DISC);
     r = CALL_CALLBACK (cmd->cb.completion, &error);
-    FREE_CALLBACK (cmd->cb.completion);
     switch (r) {
     case -1:
       if (error)
diff --git a/generator/states.c b/generator/states.c
index 263f60e..cfa9957 100644
--- a/generator/states.c
+++ b/generator/states.c
@@ -127,7 +127,6 @@ void abort_commands (struct nbd_handle *h,
 
       assert (cmd->type != NBD_CMD_DISC);
       r = CALL_CALLBACK (cmd->cb.completion, &error);
-      FREE_CALLBACK (cmd->cb.completion);
       switch (r) {
       case -1:
         if (error)
-- 
2.22.0