Re: [Libguestfs] [PATCH nbdkit 6/8] data, memory: Implement extents.

On 3/20/19 5:11 PM, Richard W.M. Jones wrote: > These plugins are both based on the same sparse array structure which > supports a simple implementation of extents. > --- > +int > +sparse_array_extents (struct sparse_array *sa, > + uint32_t count, uint64_t offset, > + struct nbdkit_extents *extents) > +{ > + uint32_t n, type; > + void *p; > + > + while (count > 0) { > + p = lookup (sa, offset, false, &n, NULL); > + if (n > count) > + n = count; Why are we clamping the information we return? I'd move this clamp...

> + > + if (p == NULL) > + type = NBDKIT_EXTENT_HOLE | NBDKIT_EXTENT_ZERO; > + else > + type = 0; /* allocated data */ > + if (nbdkit_add_extent (extents, offset, n, type) == -1) > + return -1; ...here, after we've recorded the maximum amount of information possible into the extents array. (We need the clamp to terminate the loop, but that doesn't mean we have to truncate our answer) > + > + count -= n; > + offset += n; > + } > + > + return 0; > +} Otherwise, looks good. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org