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

[Eric Blake]

On Sun, Feb 20, 2022 at 02:13:58PM +0200, Nir Soffer wrote:
> 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(-)

>        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;

This assignment is dead code after calloc,...

> -        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;

...as was this,...

> -          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;

...but keeping it here for the sake of refactoring is fine.

> +
> +  if (!zero)
> +    command->slice.buffer = create_buffer (len);
> +
> +  command->index = index;
> +
> +  return command;
> +}

ACK

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org