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 <nsof...@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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs