[Libguestfs] [PATCH nbdkit 2/2] Add insert function and use the new vector library in several places.

[Eric Blake]

On 4/19/20 4:14 PM, Richard W.M. Jones wrote:
> This extends the vector library with an insert function.  It is more
> expensive than appending, but this does not affect existing code using
> vector and can be used in new places without making those uses more
> expensive.
> 
> We use this function in nbdkit-extentlist-filter, nbdkit-eval-plugin
> and the sparse library.
> 
> This is refactoring, so should not affect functionality at all.
> However during the rewrite of eval it revealed a fencepost error in
> the loop iterating over method_scripts which is fixed here.
> ---
>   common/sparse/Makefile.am       |   1 +
>   common/utils/vector.h           |  24 +++++--
>   common/sparse/sparse.c          |  50 +++++++-------
>   plugins/eval/eval.c             |  49 ++++++--------
>   filters/extentlist/extentlist.c | 113 ++++++++++++++++----------------
>   TODO                            |   5 +-
>   6 files changed, 119 insertions(+), 123 deletions(-)

> +++ b/plugins/eval/eval.c

> @@ -104,21 +106,12 @@ compare_script (const void *methodvp, const void *entryvp)
>   static int
>   insert_method_script (const char *method, char *script)
>   {
> -  struct method_script *newp;
> -  size_t i;
>     int r;
> +  size_t i;
> +  struct method_script new_entry = { .method = method, .script = script };
>   
> -  newp = realloc (method_scripts,
> -                  sizeof (struct method_script) * (nr_method_scripts + 1));
> -  if (newp == NULL) {
> -    nbdkit_error ("insert_method_script: realloc: %m");
> -    return -1;
> -  }
> -  nr_method_scripts++;
> -  method_scripts = newp;

The old code appended the new element first, then

> -
> -  for (i = 0; i < nr_method_scripts-1; ++i) {
> -    r = compare_script (method, &method_scripts[i]);

iterated on the previously-existing elements to see if re-sorting was 
needed.

> +  for (i = 0; i < method_scripts.size; ++i) {
> +    r = compare_script (method, &method_scripts.ptr[i]);

The new code does not increase the list size until first finding where 
to insert.  I see why you called this a fencepost error, as comparing 
just the pre-/post-patch 'for' lines looks like you fixed an off-by-one 
error, but with the additional context of when the list counter was 
incremented, I don't think it is actually a bug fix, so the commit 
message had me hunting for something not there.

At any rate, the series looks good.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org