[Libguestfs] [PATCH libnbd v3 2/3] copy: Use preferred block size for copying

[Richard W.M. Jones]

On Thu, Jun 30, 2022 at 05:38:14PM +0200, Laszlo Ersek wrote:
> On 06/29/22 15:36, Richard W.M. Jones wrote:
> > + * The invariant for '*i' is always an extent which starts before or
> > + * equal to the current offset.
> > + */
> > +static bool
> > +only_zeroes (const extent_list exts, size_t *i,
> 
> (5) I think it's a bit wasteful (or at least strange) to pass in the
> entire vector structure; we certainly don't need the "cap" field. Other
> functions in libnbd seem to take pointers to vectors (which is what I'd
> expect).

Note it's only passing in 3 fields (24 bytes), not the entire array
(and I guess it should inline the whole function).  Does that change
things?

> > @@ -177,7 +229,10 @@ worker_thread (void *wp)
> >    extent_list exts = empty_vector;
> >  
> >    while (get_next_offset (&offset, &count)) {
> > +    struct command *command;
> >      size_t i;
> > +    bool is_zeroing = false;
> > +    uint64_t zeroing_start = 0;
> 
> (14) I think we should not initialize "zeroing_start". It suggests that
> the initializer value is meaningful, but it is not.

GCC warns if you don't initialize it!  It doesn't understand that
is_zeroing is related to the validity of zeroing_start.

Thanks for the other suggestions, I'll rework things.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW