[Libguestfs] [PATCH libnbd 3/8] copy: Extract create_command and create_buffer helpers

[Nir Soffer]

Creating a new command requires lot of boilerplate that makes it harder
to focus on the interesting code. Extract a helpers to create a command,
and the command slice buffer.

create_buffer is called only once, but the compiler is smart enough to
inline it, and adding it makes the code much simpler.

This change is a refactoring except fixing perror() message for calloc()
failure.

Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
 copy/multi-thread-copying.c | 87 +++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 33 deletions(-)

diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
index 2d16d2df..855d1ba4 100644
--- a/copy/multi-thread-copying.c
+++ b/copy/multi-thread-copying.c
@@ -128,20 +128,22 @@ multi_thread_copying (void)
 
   free (workers);
 }
 
 static void wait_for_request_slots (uintptr_t index);
 static unsigned in_flight (uintptr_t index);
 static void poll_both_ends (uintptr_t index);
 static int finished_read (void *vp, int *error);
 static int free_command (void *vp, int *error);
 static void fill_dst_range_with_zeroes (struct command *command);
+static struct command *create_command (uint64_t offset, size_t len, bool zero,
+                                       uintptr_t index);
 
 /* There are 'threads' worker threads, each copying work ranges from
  * src to dst until there are no more work ranges.
  */
 static void *
 worker_thread (void *indexp)
 {
   uintptr_t index = (uintptr_t) indexp;
   uint64_t offset, count;
   extent_list exts = empty_vector;
@@ -150,71 +152,43 @@ worker_thread (void *indexp)
     size_t i;
 
     assert (0 < count && count <= THREAD_WORK_SIZE);
     if (extents)
       src->ops->get_extents (src, index, offset, count, &exts);
     else
       default_get_extents (src, index, offset, count, &exts);
 
     for (i = 0; i < exts.len; ++i) {
       struct command *command;
-      struct buffer *buffer;
-      char *data;
       size_t len;
 
       if (exts.ptr[i].zero) {
         /* The source is zero so we can proceed directly to skipping,
          * fast zeroing, or writing zeroes at the destination.
          */
-        command = calloc (1, sizeof *command);
-        if (command == NULL) {
-          perror ("malloc");
-          exit (EXIT_FAILURE);
-        }
-        command->offset = exts.ptr[i].offset;
-        command->slice.len = exts.ptr[i].length;
-        command->slice.base = 0;
-        command->index = index;
+        command = create_command (exts.ptr[i].offset, exts.ptr[i].length,
+                                  true, index);
         fill_dst_range_with_zeroes (command);
       }
 
       else /* data */ {
         /* As the extent might be larger than permitted for a single
          * command, we may have to split this into multiple read
          * requests.
          */
         while (exts.ptr[i].length > 0) {
           len = exts.ptr[i].length;
           if (len > request_size)
             len = request_size;
-          data = malloc (len);
-          if (data == NULL) {
-            perror ("malloc");
-            exit (EXIT_FAILURE);
-          }
-          buffer = calloc (1, sizeof *buffer);
-          if (buffer == NULL) {
-            perror ("malloc");
-            exit (EXIT_FAILURE);
-          }
-          buffer->data = data;
-          buffer->refs = 1;
-          command = calloc (1, sizeof *command);
-          if (command == NULL) {
-            perror ("malloc");
-            exit (EXIT_FAILURE);
-          }
-          command->offset = exts.ptr[i].offset;
-          command->slice.len = len;
-          command->slice.base = 0;
-          command->slice.buffer = buffer;
-          command->index = index;
+
+          command = create_command (exts.ptr[i].offset, len,
+                                    false, index);
 
           wait_for_request_slots (index);
 
           /* Begin the asynch read operation. */
           src->ops->asynch_read (src, command,
                                  (nbd_completion_callback) {
                                    .callback = finished_read,
                                    .user_data = command,
                                  });
 
@@ -331,20 +305,67 @@ poll_both_ends (uintptr_t index)
     else if ((fds[1].revents & POLLOUT) != 0)
       dst->ops->asynch_notify_write (dst, index);
     else if ((fds[1].revents & (POLLERR | POLLNVAL)) != 0) {
       errno = ENOTCONN;
       perror (dst->name);
       exit (EXIT_FAILURE);
     }
   }
 }
 
+/* Create a new buffer. */
+static struct buffer*
+create_buffer (size_t len)
+{
+  struct buffer *buffer;
+
+  buffer = calloc (1, sizeof *buffer);
+  if (buffer == NULL) {
+    perror ("calloc");
+    exit (EXIT_FAILURE);
+  }
+
+  buffer->data = malloc (len);
+  if (buffer->data == NULL) {
+    perror ("malloc");
+    exit (EXIT_FAILURE);
+  }
+
+  buffer->refs = 1;
+
+  return buffer;
+}
+
+/* Create a new command for read or zero. */
+static struct command *
+create_command (uint64_t offset, size_t len, bool zero, uintptr_t index)
+{
+  struct command *command;
+
+  command = calloc (1, sizeof *command);
+  if (command == NULL) {
+    perror ("calloc");
+    exit (EXIT_FAILURE);
+  }
+
+  command->offset = offset;
+  command->slice.len = len;
+  command->slice.base = 0;
+
+  if (!zero)
+    command->slice.buffer = create_buffer (len);
+
+  command->index = index;
+
+  return command;
+}
+
 /* Create a sub-command of an existing command.  This creates a slice
  * referencing the buffer of the existing command without copying.
  */
 static struct command *
 create_subcommand (struct command *command, uint64_t offset, size_t len,
                    bool zero)
 {
   const uint64_t end = command->offset + command->slice.len;
   struct command *newcommand;
 
-- 
2.35.1