search for: nbdkit_extents_free

...55 insertions(+) > > + extents2 = nbdkit_extents_new (0, real_size_copy); > + if (offset + count <= real_size_copy) > + n = count; > + else > + n = real_size_copy - offset; > + if (next_ops->extents (nxdata, n, offset, flags, extents2, err) == -1) { > + nbdkit_extents_free (extents2); > + return -1; > + } > + > + for (i = 0; i < nbdkit_extents_count (extents2); ++i) { > + struct nbdkit_extent e = nbdkit_get_extent (extents2, i); > + > + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) { > + nbdkit_extents_f...

Now that cleanup.h is in common code, we can use it in our filters. The first round focuses just on places that called nbdkit_extents_free(), as all three callers had multiple exit paths that definitely benefit from the macro. Signed-off-by: Eric Blake <eblake@redhat.com> --- filters/offset/offset.c | 13 +++++-------- filters/partition/partition.c | 12 ++++-------- filters/truncate/truncate.c | 12 ++++-------- filte...

On 4/23/19 2:06 PM, Eric Blake wrote: > Now that cleanup.h is in common code, we can use it in our > filters. The first round focuses just on places that called > nbdkit_extents_free(), as all three callers had multiple exit paths > that definitely benefit from the macro. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > filters/offset/offset.c | 13 +++++-------- > filters/partition/partition.c | 12 ++++-------- > filters/truncate/tr...

I think the extents map is just too complicated and is unnecessarily so. How about instead we define the plugin interface to be: int can_extents (void *handle); // as before int extents (void *handle, uint32_t count, uint64_t offset, uint32_t flags, struct nbdkit_extents_list *list); and have the extents_list be a simple list. The first extent you add must start at offset.

...struct nbdkit_extents_map *extents_map, + int *err) +{ + struct nbdkit_extents_map *map2; + + map2 = nbdkit_extents_new (); + if (map2 == NULL) + return -1; + if (next_ops->extents (nxdata, count, offs + offset, + flags, map2, err) == -1) { + nbdkit_extents_free (map2); + return -1; + } + + /* Transform offsets in map2, return result in extents_map. */ + if (nbdkit_extents_foreach (map2, subtract_offset, extents_map, + NBDKIT_EXTENTS_FOREACH_FLAG_RANGE, + offset, range) == -1) { + nbdkit_ex...

...copy the + * returned data to the original array. + */ + extents2 = nbdkit_extents_new (0, real_size_copy); + if (offset + count <= real_size_copy) + n = count; + else + n = real_size_copy - offset; + if (next_ops->extents (nxdata, n, offset, flags, extents2, err) == -1) { + nbdkit_extents_free (extents2); + return -1; + } + + for (i = 0; i < nbdkit_extents_count (extents2); ++i) { + struct nbdkit_extent e = nbdkit_get_extent (extents2, i); + + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) { + nbdkit_extents_free (extents2); + return -1; + }...

...+ extents2 = nbdkit_extents_new (0, real_size_copy); > > + if (offset + count <= real_size_copy) > > + n = count; > > + else > > + n = real_size_copy - offset; > > + if (next_ops->extents (nxdata, n, offset, flags, extents2, err) == -1) { > > + nbdkit_extents_free (extents2); > > + return -1; > > + } > > + > > + for (i = 0; i < nbdkit_extents_count (extents2); ++i) { > > + struct nbdkit_extent e = nbdkit_get_extent (extents2, i); > > + > > + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) ==...

There's more that can be done (in particular, use of CLEANUP_FREE), but this is enough to at least see if I'm on the right track. I couldn't figure out an obvious difference between common/include and common/utils, but it looks like the former is for things that are inlineable via .h only, while the latter is when you need to link in a convenience library, so this landed in the

...is no. Consider - the earlier proposal required the offset filter to do: + struct nbdkit_extents_map *map2; + + map2 = nbdkit_extents_new (); + if (map2 == NULL) + return -1; + if (next_ops->extents (nxdata, count, offs + offset, + flags, map2, err) == -1) { + nbdkit_extents_free (map2); + return -1; + } + + /* Transform offsets in map2, return result in extents_map. */ + if (nbdkit_extents_foreach (map2, subtract_offset, extents_map, + NBDKIT_EXTENTS_FOREACH_FLAG_RANGE, + offset, range) == -1) { + nbdkit_ex...

...nts (nxdata, count, offset, flags, extents2, err); > + for (i = 0; i < nbdkit_extents_size (extents2); ++i) { > + e = nbdkit_get_extent (extents2, i); > + e.offset = /* transform offset */; > + nbdkit_add_extent (extents, e.offset, e.length, e.type); > + } > + nbdkit_extents_free (extents2); > + } > + > +More complicated transformations may require the filter to allocate a > +new extents list using C<nbdkit_extents_new> and free the old one > +using C<nbdkit_extents_free>. This sentence sounds odd after the example. You'll probably want to re...

...flags, extents2, err) == -1) + goto error; + + for (i = 0; i < nbdkit_extents_size (extents2); ++i) { + e = nbdkit_get_extent (extents2, i); + e.offset -= offset; + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) + goto error; + } + nbdkit_extents_free (extents2); + return 0; + + error: + nbdkit_extents_free (extents2); + return -1; +} + static struct nbdkit_filter filter = { .name = "offset", .longname = "nbdkit offset filter", @@ -144,6 +178,7 @@ static struct nbdkit_filter filter = { .pwri...

...flags, extents2, err) == -1) + goto error; + + for (i = 0; i < nbdkit_extents_size (extents2); ++i) { + e = nbdkit_get_extent (extents2, i); + e.offset -= h->offset; + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) + goto error; + } + nbdkit_extents_free (extents2); + return 0; + + error: + nbdkit_extents_free (extents2); + return -1; +} + static struct nbdkit_filter filter = { .name = "partition", .longname = "nbdkit partition filter", @@ -237,6 +272,7 @@ static struct nbdkit_filter filter = {...

...flags, extents2, err) == -1) + goto error; + + for (i = 0; i < nbdkit_extents_count (extents2); ++i) { + e = nbdkit_get_extent (extents2, i); + e.offset -= h->offset; + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) + goto error; + } + nbdkit_extents_free (extents2); + return 0; + + error: + nbdkit_extents_free (extents2); + return -1; +} + static struct nbdkit_filter filter = { .name = "partition", .longname = "nbdkit partition filter", @@ -237,6 +273,7 @@ static struct nbdkit_filter filter = {...

...flags, extents2, err) == -1) + goto error; + + for (i = 0; i < nbdkit_extents_count (extents2); ++i) { + e = nbdkit_get_extent (extents2, i); + e.offset -= offset; + if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) + goto error; + } + nbdkit_extents_free (extents2); + return 0; + + error: + nbdkit_extents_free (extents2); + return -1; +} + static struct nbdkit_filter filter = { .name = "offset", .longname = "nbdkit offset filter", @@ -144,6 +179,7 @@ static struct nbdkit_filter filter = { .pwri...

...ags, extents2, err); > > + for (i = 0; i < nbdkit_extents_size (extents2); ++i) { > > + e = nbdkit_get_extent (extents2, i); > > + e.offset = /* transform offset */; > > + nbdkit_add_extent (extents, e.offset, e.length, e.type); > > + } > > + nbdkit_extents_free (extents2); > > + } > > + > > +More complicated transformations may require the filter to allocate a > > +new extents list using C<nbdkit_extents_new> and free the old one > > +using C<nbdkit_extents_free>. > > This sentence sounds odd after the exam...

On 3/29/19 3:40 AM, Richard W.M. Jones wrote: >>> + nbdkit_extents_free (extents2); >>> + >> >> Should we be using the CLEANUP_EXTENTS_FREE macro here and in other filters? > > At the moment the CLEANUP_* macros are only available to server code, > not to plugins or filters. They would certainly make plugins and > filters easier to wr...

...rr) == -1) >+ goto error; >+ >+ for (i = 0; i < nbdkit_extents_count (extents2); ++i) { >+ e = nbdkit_get_extent (extents2, i); >+ e.offset -= h->offset; >+ if (nbdkit_add_extent (extents, e.offset, e.length, e.type) == -1) >+ goto error; >+ } >+ nbdkit_extents_free (extents2); >+ return 0; >+ >+ error: >+ nbdkit_extents_free (extents2); >+ return -1; >+} >+ > static struct nbdkit_filter filter = { > .name = "partition", > .longname = "nbdkit partition filter", >@@ -237,6 +273,7...

...nt64_t end) nbdkit_error ("nbdkit_extents_new: malloc: %m"); return NULL; } - r->extents = NULL; - r->nr_extents = r->allocated = 0; + r->extents = (extents) empty_vector; r->start = start; r->end = end; r->next = -1; @@ -104,7 +106,7 @@ void nbdkit_extents_free (struct nbdkit_extents *exts) { if (exts) { - free (exts->extents); + free (exts->extents.ptr); free (exts); } } @@ -112,40 +114,25 @@ nbdkit_extents_free (struct nbdkit_extents *exts) size_t nbdkit_extents_count (const struct nbdkit_extents *exts) { - return exts->n...

...underlying plugin. */ +static uint64_t size; + +/* Cached extents from the last extents () call and its start and end for the + sake of simplicity. */ +struct nbdkit_extents *cache_extents; +static uint64_t cache_start; +static uint64_t cache_end; + +static void +cacheextents_unload (void) +{ + nbdkit_extents_free (cache_extents); +} + +static int +cacheextents_add (struct nbdkit_extents *extents) +{ + size_t i = 0; + + for (i = 0; i < nbdkit_extents_count (cache_extents); i++) { + struct nbdkit_extent ex = nbdkit_get_extent (cache_extents, i); + if (nbdkit_add_extent (extents, ex.offset, ex.lengt...

...offset); + next_ops->extents (nxdata, count, offset, flags, extents2, err); + for (i = 0; i < nbdkit_extents_size (extents2); ++i) { + e = nbdkit_get_extent (extents2, i); + e.offset = /* transform offset */; + nbdkit_add_extent (extents, e.offset, e.length, e.type); + } + nbdkit_extents_free (extents2); + } + +More complicated transformations may require the filter to allocate a +new extents list using C<nbdkit_extents_new> and free the old one +using C<nbdkit_extents_free>. + +If there is an error, C<.extents> should call C<nbdkit_error> with an +error message...