On Mon, Feb 21, 2022 at 5:03 PM Eric Blake <ebl...@redhat.com> wrote: > > 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
Right, I will improve this later. > > > + > > + 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