Libguestfs - Oct 2020 - Re: [PATCH libnbd 1/5] common/utils: Copy simple vector library from nbdkit.

This adds coalescing of adjacent extents of the same type, as
mentioned by Eric Blake in the commit message here:

https://github.com/libguestfs/libnbd/commit/46072f6611f80245846a445766da071e457b00cd

The patch series is rather long because it detours through adding the
<vector.h> library from nbdkit into libnbd and replacing ad hoc uses
of realloc, char ** etc in various places.

Rich.

This library proved useful in nbdkit where we need to construct an
array or vector of arbitrary objects, with the easy ability to append
at the end.  Wherever code uses realloc(3) to build an array of
objects is a candidate for replacement by this library.
---
 Makefile.am              |   1 +
 common/utils/Makefile.am |  44 ++++++++++
 common/utils/vector.c    |  51 +++++++++++
 common/utils/vector.h    | 177 +++++++++++++++++++++++++++++++++++++++
 configure.ac             |   1 +
 5 files changed, 274 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 5037848..8e5d425 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -31,6 +31,7 @@ SUBDIRS = \
 	generator \
 	include \
 	common/include \
+	common/utils \
 	lib \
 	docs \
 	examples \
diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
new file mode 100644
index 0000000..688f827
--- /dev/null
+++ b/common/utils/Makefile.am
@@ -0,0 +1,44 @@
+# nbdkit
+# Copyright (C) 2019 Red Hat Inc.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+# * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#
+# * Redistributions in binary form must reproduce the above copyright
+# notice, this list of conditions and the following disclaimer in the
+# documentation and/or other materials provided with the distribution.
+#
+# * Neither the name of Red Hat nor the names of its contributors may be
+# used to endorse or promote products derived from this software without
+# specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+# THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+# PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+# CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+# USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+# OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+# SUCH DAMAGE.
+
+include $(top_srcdir)/common-rules.mk
+
+noinst_LTLIBRARIES = libutils.la
+
+libutils_la_SOURCES = \
+	vector.c \
+	vector.h \
+	$(NULL)
+libutils_la_CFLAGS = \
+	$(WARNINGS_CFLAGS) \
+	$(NULL)
+libutils_la_LIBADD = \
+	$(NULL)
diff --git a/common/utils/vector.c b/common/utils/vector.c
new file mode 100644
index 0000000..00cd254
--- /dev/null
+++ b/common/utils/vector.c
@@ -0,0 +1,51 @@
+/* nbdkit
+ * Copyright (C) 2018-2020 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "vector.h"
+
+int
+generic_vector_reserve (struct generic_vector *v, size_t n, size_t itemsize)
+{
+  void *newptr;
+
+  newptr = realloc (v->ptr, (n + v->alloc) * itemsize);
+  if (newptr == NULL)
+    return -1;
+  v->ptr = newptr;
+  v->alloc += n;
+  return 0;
+}
diff --git a/common/utils/vector.h b/common/utils/vector.h
new file mode 100644
index 0000000..6468fd9
--- /dev/null
+++ b/common/utils/vector.h
@@ -0,0 +1,177 @@
+/* nbdkit
+ * Copyright (C) 2020 Red Hat Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ *
+ * * Neither the name of Red Hat nor the names of its contributors may be
+ * used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS
IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
+ * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+/* Simple implementation of a vector.  It can be cheaply appended, and
+ * more expensively inserted.  There are two main use-cases we
+ * consider: lists of strings (either with a defined length, or
+ * NULL-terminated), and lists of numbers.  It is generic so could be
+ * used for lists of anything (eg. structs) where being able to append
+ * easily is important.
+ */
+
+#ifndef NBDKIT_VECTOR_H
+#define NBDKIT_VECTOR_H
+
+#include <assert.h>
+#include <string.h>
+
+/* Use of this macro defines a new type called ‘name’ containing an
+ * extensible vector of ‘type’ elements.  For example:
+ *
+ *   DEFINE_VECTOR_TYPE(string_vector, char *)
+ *
+ * defines a new type called ‘string_vector’ as a vector of ‘char *’.
+ * You can create variables of this type:
+ *
+ *   string_vector names = empty_vector;
+ *
+ * where ‘names.ptr[]’ will be an array of strings and ‘names.size’
+ * will be the number of strings.  There are no get/set accessors.  To
+ * iterate over the strings you can use the ‘.ptr’ field directly:
+ *
+ *   for (size_t i = 0; i < names.size; ++i)
+ *     printf ("%s\n", names.ptr[i]);
+ *
+ * Initializing with ‘empty_vector’ sets ‘.ptr = NULL’ and ‘.size = 0’.
+ *
+ * DEFINE_VECTOR_TYPE also defines utility functions.  For the full
+ * list see the definition below, but useful functions include:
+ *
+ * ‘name’_append  (eg. ‘string_vector_append’)
+ *   - Append a new element at the end.  This operation is cheap.
+ *
+ * ‘name’_insert  (eg. ‘string_vector_insert’)
+ *   - Insert a new element at the beginning, middle or end.  This
+ *     operation is more expensive because existing elements may need
+ *     to be copied around.
+ *
+ * Both functions extend the vector if required, and so both may fail
+ * (returning -1) which you must check for.
+ */
+#define DEFINE_VECTOR_TYPE(name, type)                                  \
+  struct name {                                                         \
+    type *ptr;                 /* Pointer to array of items. */         \
+    size_t size;               /* Number of valid items in the array. */ \
+    size_t alloc;              /* Number of items allocated. */         \
+  };                                                                    \
+  typedef struct name name;                                             \
+                                                                        \
+  /* Reserve n elements at the end of the vector.  Note space is        \
+   * allocated but the vector size is not increased and the new         \
+   * elements are not initialized.                                      \
+   */                                                                   \
+  static inline int                                                     \
+  name##_reserve (name *v, size_t n)                                    \
+  {                                                                     \
+    return generic_vector_reserve ((struct generic_vector *)v, n,       \
+                                   sizeof (type));                      \
+  }                                                                     \
+                                                                        \
+  /* Insert at i'th element.  i=0 => beginning  i=size => append */  
\
+  static inline int                                                     \
+  name##_insert (name *v, type elem, size_t i)                          \
+  {                                                                     \
+    assert (i <= v->size);                                              \
+    if (v->size >= v->alloc) {                                        
\
+      if (name##_reserve (v, 1) == -1) return -1;                       \
+    }                                                                   \
+    memmove (&v->ptr[i+1], &v->ptr[i], (v->size-i) * sizeof
(elem));    \
+    v->ptr[i] = elem;                                                   \
+    v->size++;                                                          \
+    return 0;                                                           \
+  }                                                                     \
+                                                                        \
+  /* Append a new element to the end of the vector. */                  \
+  static inline int                                                     \
+  name##_append (name *v, type elem)                                    \
+  {                                                                     \
+    return name##_insert (v, elem, v->size);                            \
+  }                                                                     \
+                                                                        \
+  /* Remove i'th element.  i=0 => beginning  i=size-1 => end */      
\
+  static inline void                                                    \
+  name##_remove (name *v, size_t i)                                     \
+  {                                                                     \
+    assert (i < v->size);                                               \
+    memmove (&v->ptr[i], &v->ptr[i+1], (v->size-i) * sizeof
(type));    \
+    v->size--;                                                          \
+  }                                                                     \
+                                                                        \
+  /* Remove all elements and deallocate the vector. */                  \
+  static inline void                                                    \
+  name##_reset (name *v)                                                \
+  {                                                                     \
+    free (v->ptr);                                                      \
+    v->ptr = NULL;                                                      \
+    v->size = v->alloc = 0;                                             \
+  }                                                                     \
+                                                                        \
+  /* Iterate over the vector, calling f() on each element. */           \
+  static inline void                                                    \
+  name##_iter (name *v, void (*f) (type elem))                          \
+  {                                                                     \
+    size_t i;                                                           \
+    for (i = 0; i < v->size; ++i)                                       \
+      f (v->ptr[i]);                                                    \
+  }                                                                     \
+                                                                        \
+  /* Sort the elements of the vector. */                                \
+  static inline void                                                    \
+  name##_sort (name *v,                                                 \
+               int (*compare) (const type *p1, const type *p2))         \
+  {                                                                     \
+    qsort (v->ptr, v->size, sizeof (type), (void *) compare);           \
+  }                                                                     \
+                                                                        \
+  /* Search for an exactly matching element in the vector using a       \
+   * binary search.  Returns a pointer to the element or NULL.          \
+   */                                                                   \
+  static inline type *                                                  \
+  name##_search (const name *v, const void *key,                        \
+                 int (*compare) (const void *key, const type *v))       \
+  {                                                                     \
+    return bsearch (key, v->ptr, v->size, sizeof (type),                \
+                    (void *) compare);                                  \
+  }
+
+#define empty_vector { .ptr = NULL, .size = 0, .alloc = 0 }
+
+struct generic_vector {
+  void *ptr;
+  size_t size;
+  size_t alloc;
+};
+
+extern int generic_vector_reserve (struct generic_vector *v,
+                                   size_t n, size_t itemsize);
+
+#endif /* NBDKIT_VECTOR_H */
diff --git a/configure.ac b/configure.ac
index 8d99042..1322f81 100644
--- a/configure.ac
+++ b/configure.ac
@@ -450,6 +450,7 @@ AC_CONFIG_FILES([sh/nbdsh],
 AC_CONFIG_FILES([Makefile
                  bash/Makefile
                  common/include/Makefile
+                 common/utils/Makefile
                  copy/Makefile
                  docs/Makefile
                  examples/Makefile
-- 
2.29.0.rc2

This allows us to remove two unused internal functions handling string
lists.
---
 generator/states-connect-socket-activation.c | 59 ++++++-------
 generator/states-connect.c                   |  8 +-
 generator/states-newstyle-opt-meta-context.c | 22 ++---
 lib/Makefile.am                              |  2 +
 lib/connect.c                                | 22 +----
 lib/crypto.c                                 | 36 ++++----
 lib/handle.c                                 | 28 +++---
 lib/internal.h                               | 10 ++-
 lib/uri.c                                    | 89 +++++++++-----------
 lib/utils.c                                  | 43 +++-------
 10 files changed, 133 insertions(+), 186 deletions(-)

diff --git a/generator/states-connect-socket-activation.c
b/generator/states-connect-socket-activation.c
index ee08dff..5f4cfbd 100644
--- a/generator/states-connect-socket-activation.c
+++ b/generator/states-connect-socket-activation.c
@@ -47,11 +47,11 @@ extern char **environ;
 static char **
 prepare_socket_activation_environment (void)
 {
-  char **env = NULL;
+  string_vector env = empty_vector;
   char *p0 = NULL, *p1 = NULL;
-  size_t i, len;
-  void *vp;
+  size_t i;
 
+  /* Reserve slots env[0] and env[1]. */
   p0 = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
   if (p0 == NULL)
     goto err;
@@ -59,38 +59,35 @@ prepare_socket_activation_environment (void)
   if (p1 == NULL)
     goto err;
 
-  /* Copy the current environment. */
-  env = nbd_internal_copy_string_list (environ);
-  if (env == NULL)
+  if (string_vector_append (&env, p0) == -1 ||
+      string_vector_append (&env, p1) == -1)
     goto err;
 
-  /* Reserve slots env[0] and env[1]. */
-  len = nbd_internal_string_list_length (env);
-  vp = realloc (env,
-                sizeof (char *) * (len + 3)); /* include final NULL entry */
-  if (vp == NULL)
-    goto err;
-  env = vp;
-  memmove (&env[2], &env[0], sizeof (char *) * (len + 1));
-
-  env[0] = p0;          /* Code below assumes env[0] is LISTEN_PID. */
-  env[1] = p1;
-
-  /* Remove any existing LISTEN_PID or LISTEN_FDS instances. */
-  for (i = 2; env[i] != NULL; ++i) {
-    if (strncmp (env[i], "LISTEN_PID=", PREFIX_LENGTH) == 0 ||
-        strncmp (env[i], "LISTEN_FDS=", PREFIX_LENGTH) == 0) {
-      memmove (&env[i], &env[i+1],
-               sizeof (char *) * (nbd_internal_string_list_length
(&env[i])));
-      i--;
+  /* Append the current environment, but remove LISTEN_PID, LISTEN_FDS. */
+  for (i = 0; environ[i] != NULL; ++i) {
+    if (strncmp (environ[i], "LISTEN_PID=", PREFIX_LENGTH) != 0
&&
+        strncmp (environ[i], "LISTEN_FDS=", PREFIX_LENGTH) != 0) {
+      char *copy = strdup (environ[i]);
+      if (copy == NULL)
+        goto err;
+      if (string_vector_append (&env, copy) == -1) {
+        free (copy);
+        goto err;
+      }
     }
   }
 
-  return env;
+  /* The environ must be NULL-terminated. */
+  if (string_vector_append (&env, NULL) == -1)
+    goto err;
+
+  return env.ptr;
 
  err:
   set_error (errno, "malloc");
-  nbd_internal_free_string_list (env);
+  for (i = 2; env.ptr[i] != NULL; ++i)
+    free (env.ptr[i]);
+  free (env.ptr);
   free (p0);
   free (p1);
   return NULL;
@@ -105,8 +102,8 @@ STATE_MACHINE {
   int flags;
 
   assert (!h->sock);
-  assert (h->argv);
-  assert (h->argv[0]);
+  assert (h->argv.ptr);
+  assert (h->argv.ptr[0]);
 
   /* Use /tmp instead of TMPDIR because we must ensure the path is
    * short enough to store in the sockaddr_un.  On some platforms this
@@ -201,8 +198,8 @@ STATE_MACHINE {
     signal (SIGPIPE, SIG_DFL);
 
     environ = env;
-    execvp (h->argv[0], h->argv);
-    nbd_internal_fork_safe_perror (h->argv[0]);
+    execvp (h->argv.ptr[0], h->argv.ptr);
+    nbd_internal_fork_safe_perror (h->argv.ptr[0]);
     if (errno == ENOENT)
       _exit (127);
     else
diff --git a/generator/states-connect.c b/generator/states-connect.c
index 33ae844..392879d 100644
--- a/generator/states-connect.c
+++ b/generator/states-connect.c
@@ -210,8 +210,8 @@ STATE_MACHINE {
   int flags;
 
   assert (!h->sock);
-  assert (h->argv);
-  assert (h->argv[0]);
+  assert (h->argv.ptr);
+  assert (h->argv.ptr[0]);
   if (socketpair (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, sv) == -1) {
     SET_NEXT_STATE (%.DEAD);
     set_error (errno, "socketpair");
@@ -237,8 +237,8 @@ STATE_MACHINE {
     /* Restore SIGPIPE back to SIG_DFL. */
     signal (SIGPIPE, SIG_DFL);
 
-    execvp (h->argv[0], h->argv);
-    nbd_internal_fork_safe_perror (h->argv[0]);
+    execvp (h->argv.ptr[0], h->argv.ptr);
+    nbd_internal_fork_safe_perror (h->argv.ptr[0]);
     if (errno == ENOENT)
       _exit (127);
     else
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index fe82252..bbf155e 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -20,7 +20,7 @@
 
 STATE_MACHINE {
  NEWSTYLE.OPT_META_CONTEXT.START:
-  size_t i, nr_queries;
+  size_t i;
   uint32_t len, opt;
 
   /* If the server doesn't support SRs then we must skip this group.
@@ -38,8 +38,7 @@ STATE_MACHINE {
   else {
     assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
     opt = NBD_OPT_SET_META_CONTEXT;
-    if (!h->structured_replies ||
-        nbd_internal_string_list_length (h->request_meta_contexts) == 0) {
+    if (!h->structured_replies || h->request_meta_contexts.size == 0) {
       SET_NEXT_STATE (%^OPT_GO.START);
       return 0;
     }
@@ -49,9 +48,8 @@ STATE_MACHINE {
 
   /* Calculate the length of the option request data. */
   len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
-  nr_queries = nbd_internal_string_list_length (h->request_meta_contexts);
-  for (i = 0; i < nr_queries; ++i)
-    len += 4 /* length of query */ + strlen (h->request_meta_contexts[i]);
+  for (i = 0; i < h->request_meta_contexts.size; ++i)
+    len += 4 /* length of query */ + strlen
(h->request_meta_contexts.ptr[i]);
 
   h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
   h->sbuf.option.option = htobe32 (opt);
@@ -89,8 +87,7 @@ STATE_MACHINE {
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
   case 0:
-    h->sbuf.nrqueries -      htobe32 (nbd_internal_string_list_length
(h->request_meta_contexts));
+    h->sbuf.nrqueries = htobe32 (h->request_meta_contexts.size);
     h->wbuf = &h->sbuf;
     h->wlen = sizeof h->sbuf.nrqueries;
     h->wflags = MSG_MORE;
@@ -108,13 +105,12 @@ STATE_MACHINE {
   return 0;
 
  NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
-  const char *query = !h->request_meta_contexts ? NULL
-    : h->request_meta_contexts[h->querynum];
-
-  if (query == NULL) { /* end of list of requested meta contexts */
+  if (h->querynum >= h->request_meta_contexts.size) {
+    /* end of list of requested meta contexts */
     SET_NEXT_STATE (%PREPARE_FOR_REPLY);
     return 0;
   }
+  const char *query = h->request_meta_contexts.ptr[h->querynum];
 
   h->sbuf.len = htobe32 (strlen (query));
   h->wbuf = &h->sbuf.len;
@@ -124,7 +120,7 @@ STATE_MACHINE {
   return 0;
 
  NEWSTYLE.OPT_META_CONTEXT.SEND_QUERYLEN:
-  const char *query = h->request_meta_contexts[h->querynum];
+  const char *query = h->request_meta_contexts.ptr[h->querynum];
 
   switch (send_from_wbuf (h)) {
   case -1: SET_NEXT_STATE (%.DEAD); return 0;
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 9fd6331..968e41a 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -62,6 +62,7 @@ libnbd_la_SOURCES = \
 libnbd_la_CPPFLAGS = \
 	-I$(top_srcdir)/include \
 	-I$(top_srcdir)/common/include \
+	-I$(top_srcdir)/common/utils \
 	-Dsysconfdir=\"$(sysconfdir)\" \
 	$(NULL)
 libnbd_la_CFLAGS = \
@@ -71,6 +72,7 @@ libnbd_la_CFLAGS = \
 	$(LIBXML2_CFLAGS) \
 	$(NULL)
 libnbd_la_LIBADD = \
+	$(top_builddir)/common/utils/libutils.la \
 	$(GNUTLS_LIBS) \
 	$(LIBXML2_LIBS) \
 	$(NULL)
diff --git a/lib/connect.c b/lib/connect.c
index 7e42b14..99781cd 100644
--- a/lib/connect.c
+++ b/lib/connect.c
@@ -249,18 +249,11 @@ nbd_unlocked_aio_connect_socket (struct nbd_handle *h, int
sock)
 int
 nbd_unlocked_aio_connect_command (struct nbd_handle *h, char **argv)
 {
-  char **copy;
-
-  copy = nbd_internal_copy_string_list (argv);
-  if (!copy) {
-    set_error (errno, "copy_string_list");
+  if (nbd_internal_set_argv (&h->argv, argv) == -1) {
+    set_error (errno, "realloc");
     return -1;
   }
 
-  if (h->argv)
-    nbd_internal_free_string_list (h->argv);
-  h->argv = copy;
-
   return nbd_internal_run (h, cmd_connect_command);
 }
 
@@ -268,17 +261,10 @@ int
 nbd_unlocked_aio_connect_systemd_socket_activation (struct nbd_handle *h,
                                                     char **argv)
 {
-  char **copy;
-
-  copy = nbd_internal_copy_string_list (argv);
-  if (!copy) {
-    set_error (errno, "copy_string_list");
+  if (nbd_internal_set_argv (&h->argv, argv) == -1) {
+    set_error (errno, "realloc");
     return -1;
   }
 
-  if (h->argv)
-    nbd_internal_free_string_list (h->argv);
-  h->argv = copy;
-
   return nbd_internal_run (h, cmd_connect_sa);
 }
diff --git a/lib/crypto.c b/lib/crypto.c
index 516651c..a9b3789 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -31,6 +31,7 @@
 #endif
 
 #include "internal.h"
+#include "vector.h"
 
 int
 nbd_unlocked_set_tls (struct nbd_handle *h, int tls)
@@ -107,11 +108,13 @@ nbd_unlocked_set_tls_username (struct nbd_handle *h, const
char *username)
   return 0;
 }
 
+DEFINE_VECTOR_TYPE (string, char)
+
 char *
 nbd_unlocked_get_tls_username (struct nbd_handle *h)
 {
   char *s, *ret;
-  size_t len;
+  string str = empty_vector;
 
   if (h->tls_username) {
     ret = strdup (h->tls_username);
@@ -137,31 +140,22 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
     return ret;
   }
 
-  len = 16;
-  ret = NULL;
-  while (ret == NULL) {
-    char *oldret = ret;
-
-    ret = realloc (oldret, len);
-    if (ret == NULL) {
-      set_error (errno, "realloc");
-      free (oldret);
-      return NULL;
+  for (;;) {
+    if (getlogin_r (str.ptr, str.alloc) == 0) {
+      return str.ptr;
     }
-
-    if (getlogin_r (ret, len) != 0) {
-      if (errno == ERANGE) {
-        /* Try again with a larger buffer. */
-        len *= 2;
-        continue;
-      }
+    else if (errno != ERANGE) {
       set_error (errno, "getlogin_r");
-      free (ret);
+      free (str.ptr);
+      return NULL;
+    }
+    /* Try again with a larger buffer. */
+    if (string_reserve (&str, str.alloc == 0 ? 16 : str.alloc * 2) == -1) {
+      set_error (errno, "realloc");
+      free (str.ptr);
       return NULL;
     }
   }
-
-  return ret;
 }
 
 int
diff --git a/lib/handle.c b/lib/handle.c
index 35fe066..841514a 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -129,7 +129,8 @@ nbd_close (struct nbd_handle *h)
   free_cmd_list (h->cmds_to_issue);
   free_cmd_list (h->cmds_in_flight);
   free_cmd_list (h->cmds_done);
-  nbd_internal_free_string_list (h->argv);
+  string_vector_iter (&h->argv, (void *) free);
+  free (h->argv.ptr);
   if (h->sa_sockpath) {
     if (h->pid > 0)
       kill (h->pid, SIGTERM);
@@ -153,7 +154,8 @@ nbd_close (struct nbd_handle *h)
   free (h->tls_certificates);
   free (h->tls_username);
   free (h->tls_psk_file);
-  nbd_internal_free_string_list (h->request_meta_contexts);
+  string_vector_iter (&h->request_meta_contexts, (void *) free);
+  free (h->request_meta_contexts.ptr);
   free (h->hname);
   pthread_mutex_destroy (&h->lock);
   free (h);
@@ -285,8 +287,6 @@ int
 nbd_unlocked_add_meta_context (struct nbd_handle *h, const char *name)
 {
   char *copy;
-  size_t len;
-  char **list;
 
   if (strnlen (name, NBD_MAX_STRING + 1) > NBD_MAX_STRING) {
     set_error (ENAMETOOLONG, "meta context name too long for NBD
protocol");
@@ -298,17 +298,12 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const
char *name)
     set_error (errno, "strdup");
     return -1;
   }
-  len = nbd_internal_string_list_length (h->request_meta_contexts);
-  list = realloc (h->request_meta_contexts,
-                  sizeof (char *) * (len+2 /* + new entry + NULL */));
-  if (list == NULL) {
+
+  if (string_vector_append (&h->request_meta_contexts, copy) == -1) {
     free (copy);
     set_error (errno, "realloc");
     return -1;
   }
-  h->request_meta_contexts = list;
-  list[len] = copy;
-  list[len+1] = NULL;
 
   return 0;
 }
@@ -316,21 +311,20 @@ nbd_unlocked_add_meta_context (struct nbd_handle *h, const
char *name)
 ssize_t
 nbd_unlocked_get_nr_meta_contexts (struct nbd_handle *h)
 {
-  return nbd_internal_string_list_length (h->request_meta_contexts);
+  return h->request_meta_contexts.size;
 }
 
 char *
 nbd_unlocked_get_meta_context (struct nbd_handle *h, size_t i)
 {
-  size_t len = nbd_internal_string_list_length (h->request_meta_contexts);
   char *ret;
 
-  if (i >= len) {
+  if (i >= h->request_meta_contexts.size) {
     set_error (EINVAL, "meta context request out of range");
     return NULL;
   }
 
-  ret = strdup (h->request_meta_contexts[i]);
+  ret = strdup (h->request_meta_contexts.ptr[i]);
   if (ret == NULL)
     set_error (errno, "strdup");
 
@@ -340,8 +334,8 @@ nbd_unlocked_get_meta_context (struct nbd_handle *h, size_t
i)
 int
 nbd_unlocked_clear_meta_contexts (struct nbd_handle *h)
 {
-  nbd_internal_free_string_list (h->request_meta_contexts);
-  h->request_meta_contexts = NULL;
+  string_vector_iter (&h->request_meta_contexts, (void *) free);
+  string_vector_reset (&h->request_meta_contexts);
   return 0;
 }
 
diff --git a/lib/internal.h b/lib/internal.h
index ad1eeb9..d4f324e 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -40,6 +40,7 @@
 #include "byte-swapping.h"
 #include "states.h"
 #include "unlocked.h"
+#include "vector.h"
 
 /* Define unlikely macro, but only for GCC.  These are used to move
  * debug and error handling code out of hot paths, making the hot path
@@ -82,6 +83,8 @@ struct command_cb {
   nbd_completion_callback completion;
 };
 
+DEFINE_VECTOR_TYPE (string_vector, char *)
+
 struct nbd_handle {
   /* Unique name assigned to this handle for debug messages
    * (to avoid having to print actual pointers).
@@ -102,7 +105,7 @@ struct nbd_handle {
 
   /* Desired metadata contexts. */
   bool request_sr;
-  char **request_meta_contexts;
+  string_vector request_meta_contexts;
 
   /* Allowed in URIs, see lib/uri.c. */
   uint32_t uri_allow_transports;
@@ -249,7 +252,7 @@ struct nbd_handle {
    * the subprocess so we can wait on it when the connection is
    * closed.
    */
-  char **argv;
+  string_vector argv;
   pid_t pid;
 
   /* When using systemd socket activation, this directory and socket
@@ -454,9 +457,8 @@ extern int nbd_internal_aio_get_direction (enum state
state);
 
 /* utils.c */
 extern void nbd_internal_hexdump (const void *data, size_t len, FILE *fp);
-extern size_t nbd_internal_string_list_length (char **argv);
-extern char **nbd_internal_copy_string_list (char **argv);
 extern void nbd_internal_free_string_list (char **argv);
+extern int nbd_internal_set_argv (string_vector *v, char **argv);
 extern const char *nbd_internal_fork_safe_itoa (long v, char *buf, size_t len);
 extern void nbd_internal_fork_safe_perror (const char *s);
 extern char *nbd_internal_printable_buffer (const void *buf, size_t count);
diff --git a/lib/uri.c b/lib/uri.c
index 79e9b41..9f5a290 100644
--- a/lib/uri.c
+++ b/lib/uri.c
@@ -27,6 +27,7 @@
 #include <assert.h>
 
 #include "internal.h"
+#include "vector.h"
 
 #ifdef HAVE_LIBXML2
 
@@ -47,23 +48,24 @@ struct uri_query {
   char *value;
 };
 
+DEFINE_VECTOR_TYPE (uri_query_list, struct uri_query)
+
 /* Parse the query_raw substring of a URI into a list of decoded queries.
- * Return the length of the list, or -1 on error.
+ * Return 0 on success or -1 on error.
  */
 static int
-parse_uri_queries (const char *query_raw, struct uri_query **list)
+parse_uri_queries (const char *query_raw, uri_query_list *list)
 {
   /* Borrows from libvirt's viruri.c:virURIParseParams() */
   const char *end, *eq;
   const char *query = query_raw;
-  int nqueries = 0;
-  struct uri_query *tmp;
+  size_t i;
 
   if (!query || !*query)
     return 0;
 
   while (*query) {
-    char *name = NULL, *value = NULL;
+    struct uri_query q = {0};
 
     /* Find the next separator, or end of the string. */
     end = strchr (query, '&');
@@ -83,14 +85,14 @@ parse_uri_queries (const char *query_raw, struct uri_query
**list)
       /* If there is no '=' character, then we have just
"name"
        * and consistent with CGI.pm we assume value is "".
        */
-      name = xmlURIUnescapeString (query, end - query, NULL);
-      if (!name) goto error;
+      q.name = xmlURIUnescapeString (query, end - query, NULL);
+      if (!q.name) goto error;
     } else if (eq+1 == end) {
       /* Or if we have "name=" here (works around annoying
        * problem when calling xmlURIUnescapeString with len = 0).
        */
-      name = xmlURIUnescapeString (query, eq - query, NULL);
-      if (!name) goto error;
+      q.name = xmlURIUnescapeString (query, eq - query, NULL);
+      if (!q.name) goto error;
     } else if (query == eq) {
       /* If the '=' character is at the beginning then we have
        * "=value" and consistent with CGI.pm we _ignore_ this.
@@ -98,50 +100,43 @@ parse_uri_queries (const char *query_raw, struct uri_query
**list)
       goto next;
     } else {
       /* Otherwise it's "name=value". */
-      name = xmlURIUnescapeString (query, eq - query, NULL);
-      if (!name)
+      q.name = xmlURIUnescapeString (query, eq - query, NULL);
+      if (!q.name)
         goto error;
-      value = xmlURIUnescapeString (eq+1, end - (eq+1), NULL);
-      if (!value) {
-        free (name);
+      q.value = xmlURIUnescapeString (eq+1, end - (eq+1), NULL);
+      if (!q.value) {
+        free (q.name);
         goto error;
       }
     }
-    if (!value) {
-      value = strdup ("");
-      if (!value) {
-        free (name);
+    if (!q.value) {
+      q.value = strdup ("");
+      if (!q.value) {
+        free (q.name);
         goto error;
       }
     }
 
-    /* Append to the parameter set. */
-    tmp = realloc (*list, sizeof (*tmp) * (nqueries + 1));
-    if (tmp == NULL) {
-      free (name);
-      free (value);
+    /* Append to the list of queries. */
+    if (uri_query_list_append (list, q) == -1) {
+      free (q.name);
+      free (q.value);
       goto error;
     }
-    *list = tmp;
-    tmp[nqueries].name = name;
-    tmp[nqueries].value = value;
-    nqueries++;
 
   next:
     query = end;
     if (*query) query++; /* skip '&' separator */
   }
 
-  return nqueries;
+  return 0;
 
 error:
-  tmp = *list;
-  while (nqueries-- > 0) {
-    free (tmp[nqueries].name);
-    free (tmp[nqueries].value);
+  for (i = 0; i < list->size; ++list) {
+    free (list->ptr[i].name);
+    free (list->ptr[i].value);
   }
-  free (tmp);
-  *list = NULL;
+  uri_query_list_reset (list);
   return -1;
 }
 
@@ -151,8 +146,7 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
   xmlURIPtr uri = NULL;
   enum { tcp, unix_sock, vsock } transport;
   bool tls, socket_required;
-  struct uri_query *queries = NULL;
-  int nqueries = -1;
+  uri_query_list queries = empty_vector;
   int i, r;
   int ret = -1;
   const char *unixsocket = NULL;
@@ -164,8 +158,7 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
     set_error (EINVAL, "unable to parse URI: %s", raw_uri);
     goto cleanup;
   }
-  nqueries = parse_uri_queries (uri->query_raw, &queries);
-  if (nqueries == -1) {
+  if (parse_uri_queries (uri->query_raw, &queries) == -1) {
     set_error (EINVAL, "unable to parse URI queries: %s",
uri->query_raw);
     goto cleanup;
   }
@@ -235,9 +228,9 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
   }
 
   /* Parse the socket parameter. */
-  for (i = 0; i < nqueries; i++) {
-    if (strcmp (queries[i].name, "socket") == 0)
-      unixsocket = queries[i].value;
+  for (i = 0; i < queries.size; i++) {
+    if (strcmp (queries.ptr[i].name, "socket") == 0)
+      unixsocket = queries.ptr[i].value;
   }
 
   if (socket_required && !unixsocket) {
@@ -257,15 +250,15 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
     goto cleanup;
 
   /* Look for some tls-* parameters.  XXX More to come. */
-  for (i = 0; i < nqueries; i++) {
-    if (strcmp (queries[i].name, "tls-psk-file") == 0) {
+  for (i = 0; i < queries.size; i++) {
+    if (strcmp (queries.ptr[i].name, "tls-psk-file") == 0) {
       if (! h->uri_allow_local_file) {
         set_error (EPERM,
                    "local file access (tls-psk-file) is not allowed,
"
                    "call nbd_set_uri_allow_local_file to enable
this");
         goto cleanup;
       }
-      if (nbd_unlocked_set_tls_psk_file (h, queries[i].value) == -1)
+      if (nbd_unlocked_set_tls_psk_file (h, queries.ptr[i].value) == -1)
         goto cleanup;
     }
   }
@@ -333,11 +326,11 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
   ret = 0;
 
 cleanup:
-  while (nqueries-- > 0) {
-    free (queries[nqueries].name);
-    free (queries[nqueries].value);
+  for (i = 0; i < queries.size; ++i) {
+    free (queries.ptr[i].name);
+    free (queries.ptr[i].value);
   }
-  free (queries);
+  free (queries.ptr);
   xmlFreeURI (uri);
   return ret;
 }
diff --git a/lib/utils.c b/lib/utils.c
index 6e5bdeb..c3f4c4c 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -52,42 +52,25 @@ nbd_internal_hexdump (const void *data, size_t len, FILE
*fp)
   }
 }
 
-size_t
-nbd_internal_string_list_length (char **argv)
+/* Replace the string_vector with a deep copy of argv (including final NULL).
*/
+int
+nbd_internal_set_argv (string_vector *v, char **argv)
 {
-  size_t ret;
+  size_t i;
 
-  if (argv == NULL)
-    return 0;
+  string_vector_reset (v);
 
-  for (ret = 0; argv[ret] != NULL; ++ret)
-    ;
-  return ret;
-}
-
-char **
-nbd_internal_copy_string_list (char **argv)
-{
-  size_t i, j, n;
-  char **ret;
-
-  n = nbd_internal_string_list_length (argv);
-  ret = calloc (n+1, sizeof (char *));
-  if (!ret)
-    return NULL;
-
-  for (i = 0; i < n; ++i) {
-    ret[i] = strdup (argv[i]);
-    if (ret[i] == NULL) {
-      for (j = 0; j < i; ++j)
-        free (ret[j]);
-      free (ret);
-      return NULL;
+  for (i = 0; argv[i] != NULL; ++i) {
+    char *copy = strdup (argv[i]);
+    if (copy == NULL)
+      return -1;
+    if (string_vector_append (v, copy) == -1) {
+      free (copy);
+      return -1;
     }
   }
-  ret[n] = NULL;
 
-  return ret;
+  return string_vector_append (v, NULL);
 }
 
 void
-- 
2.29.0.rc2

---
 info/Makefile.am |   6 ++-
 info/nbdinfo.c   | 110 ++++++++++++++++++-----------------------------
 2 files changed, 47 insertions(+), 69 deletions(-)

diff --git a/info/Makefile.am b/info/Makefile.am
index 3fce0cd..8303658 100644
--- a/info/Makefile.am
+++ b/info/Makefile.am
@@ -46,11 +46,15 @@ LOG_COMPILER = $(top_builddir)/run
 TESTS  
 nbdinfo_SOURCES = nbdinfo.c
-nbdinfo_CPPFLAGS = -I$(top_srcdir)/include
+nbdinfo_CPPFLAGS = \
+	-I$(top_srcdir)/include \
+	-I$(top_srcdir)/common/utils \
+	$(NULL)
 nbdinfo_CFLAGS = \
 	$(WARNINGS_CFLAGS) \
 	$(NULL)
 nbdinfo_LDADD = \
+	$(top_builddir)/common/utils/libutils.la \
 	$(top_builddir)/lib/libnbd.la \
 	$(NULL)
 
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 2b22f51..532f7d4 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -31,8 +31,12 @@
 
 #include <libnbd.h>
 
+#include "vector.h"
+
 #define MIN(a,b) ((a) < (b) ? (a) : (b))
 
+DEFINE_VECTOR_TYPE (string_vector, char *)
+
 static const char *progname;
 static FILE *fp;
 static bool list_all = false;
@@ -41,16 +45,12 @@ static bool json_output = false;
 static const char *map = NULL;
 static bool size_only = false;
 
-struct context_list {
+struct export {
   char *name;
-  struct context_list *next;
+  char *desc;
 };
-
-static struct export_list {
-  size_t len;
-  char **names;
-  char **descs;
-} export_list;
+DEFINE_VECTOR_TYPE (exports, struct export)
+static exports export_list = empty_vector;
 
 static int collect_context (void *opaque, const char *name);
 static int collect_export (void *opaque, const char *name,
@@ -246,8 +246,8 @@ main (int argc, char *argv[])
   }
 
   if (list_all) {               /* --list */
-    if (nbd_opt_list (nbd, (nbd_list_callback) {
-          .callback = collect_export, .user_data = &export_list}) == -1) {
+    if (nbd_opt_list (nbd,
+                      (nbd_list_callback) {.callback = collect_export}) == -1)
{
       fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
       exit (EXIT_FAILURE);
     }
@@ -342,12 +342,11 @@ main (int argc, char *argv[])
       fprintf (fp, "}\n");
   }
 
-  for (i = 0; i < export_list.len; i++) {
-    free (export_list.names[i]);
-    free (export_list.descs[i]);
+  for (i = 0; i < export_list.size; ++i) {
+    free (export_list.ptr[i].name);
+    free (export_list.ptr[i].desc);
   }
-  free (export_list.names);
-  free (export_list.descs);
+  free (export_list.ptr);
   nbd_opt_abort (nbd);
   nbd_shutdown (nbd, 0);
   nbd_close (nbd);
@@ -370,52 +369,30 @@ main (int argc, char *argv[])
 static int
 collect_context (void *opaque, const char *name)
 {
-  struct context_list **head = opaque;
-  struct context_list *next = malloc (sizeof *next);
+  string_vector *contexts = opaque;
+  char *copy;
 
-  if (!next) {
+  copy = strdup (name);
+  if (copy == NULL || string_vector_append (contexts, copy) == -1) {
     perror ("malloc");
     exit (EXIT_FAILURE);
   }
-  next->name = strdup (name);
-  if (!next->name) {
-    perror ("strdup");
-    exit (EXIT_FAILURE);
-  }
-  next->next = *head;
-  *head = next;
   return 0;
 }
 
 static int
 collect_export (void *opaque, const char *name, const char *desc)
 {
-  struct export_list *l = opaque;
-  char **names, **descs;
+  struct export e;
 
-  names = realloc (l->names, (l->len + 1) * sizeof name);
-  descs = realloc (l->descs, (l->len + 1) * sizeof desc);
-  if (!names || !descs) {
-    perror ("realloc");
+  e.name = strdup (name);
+  e.desc = strdup (desc);
+  if (e.name == NULL || e.desc == NULL ||
+      exports_append (&export_list, e) == -1) {
+    perror ("malloc");
     exit (EXIT_FAILURE);
   }
-  l->names = names;
-  l->descs = descs;
-  l->names[l->len] = strdup (name);
-  if (!l->names[l->len]) {
-    perror ("strdup");
-    exit (EXIT_FAILURE);
-  }
-  if (*desc) {
-    l->descs[l->len] = strdup (desc);
-    if (!l->descs[l->len]) {
-      perror ("strdup");
-      exit (EXIT_FAILURE);
-    }
-  }
-  else
-    l->descs[l->len] = NULL;
-  l->len++;
+
   return 0;
 }
 
@@ -423,7 +400,7 @@ static void
 list_one_export (struct nbd_handle *nbd, const char *desc,
                  bool first, bool last)
 {
-  int64_t size;
+  int64_t i, size;
   char *export_name = NULL;
   char *export_desc = NULL;
   char *content = NULL;
@@ -431,7 +408,7 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
   int can_cache, can_df, can_fast_zero, can_flush, can_fua,
     can_multi_conn, can_trim, can_zero;
   int64_t block_minimum, block_preferred, block_maximum;
-  struct context_list *contexts = NULL;
+  string_vector contexts = empty_vector;
   bool show_context = false;
 
   /* Collect the metadata we are going to display. If opt_info works,
@@ -474,8 +451,9 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
   block_minimum = nbd_get_block_size (nbd, LIBNBD_SIZE_MINIMUM);
   block_preferred = nbd_get_block_size (nbd, LIBNBD_SIZE_PREFERRED);
   block_maximum = nbd_get_block_size (nbd, LIBNBD_SIZE_MAXIMUM);
-  if (nbd_opt_list_meta_context (nbd, (nbd_context_callback) {
-        .callback = collect_context, .user_data = &contexts}) != -1)
+  if (nbd_opt_list_meta_context (nbd,
+             (nbd_context_callback) {.callback = collect_context,
+                                       .user_data = &contexts}) != -1)
     show_context = true;
 
   /* Get content last, as it moves the connection out of negotiating */
@@ -493,8 +471,8 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
       fprintf (fp, "\tcontent: %s\n", content);
     if (show_context) {
       fprintf (fp, "\tcontexts:\n");
-      for (struct context_list *next = contexts; next; next = next->next)
-        fprintf (fp, "\t\t%s\n", next->name);
+      for (i = 0; i < contexts.size; ++i)
+        fprintf (fp, "\t\t%s\n", contexts.ptr[i]);
     }
     if (is_rotational >= 0)
       fprintf (fp, "\t%s: %s\n", "is_rotational",
@@ -551,10 +529,10 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
 
     if (show_context) {
       fprintf (fp, "\t\"contexts\": [\n");
-      for (struct context_list *next = contexts; next; next = next->next) {
+      for (i = 0; i < contexts.size; ++i) {
         fprintf (fp, "\t\t");
-        print_json_string (next->name);
-        if (next->next)
+        print_json_string (contexts.ptr[i]);
+        if (i+1 != contexts.size)
           fputc (',', fp);
         fputc ('\n', fp);
       }
@@ -611,12 +589,8 @@ list_one_export (struct nbd_handle *nbd, const char *desc,
       fprintf (fp, "\t},\n");
   }
 
-  while (contexts) {
-    struct context_list *next = contexts->next;
-    free (contexts->name);
-    free (contexts);
-    contexts = next;
-  }
+  string_vector_iter (&contexts, (void *) free);
+  free (contexts.ptr);
   free (content);
   free (export_name);
   free (export_desc);
@@ -627,11 +601,11 @@ list_all_exports (struct nbd_handle *nbd1, const char
*uri)
 {
   size_t i;
 
-  if (export_list.len == 0 && json_output)
+  if (export_list.size == 0 && json_output)
     fprintf (fp, "\"exports\": []\n");
 
-  for (i = 0; i < export_list.len; ++i) {
-    const char *name = export_list.names[i];
+  for (i = 0; i < export_list.size; ++i) {
+    const char *name = export_list.ptr[i].name;
     struct nbd_handle *nbd2;
 
     if (probe_content) {
@@ -660,8 +634,8 @@ list_all_exports (struct nbd_handle *nbd1, const char *uri)
     }
 
     /* List the metadata of this export. */
-    list_one_export (nbd2, export_list.descs[i], i == 0,
-                     i + 1 == export_list.len);
+    list_one_export (nbd2, export_list.ptr[i].desc, i == 0,
+                     i + 1 == export_list.size);
 
     if (probe_content) {
       nbd_shutdown (nbd2, 0);
-- 
2.29.0.rc2

This refactoring simply changes the code so that the extent callback
builds up a list of extents (using a vector of uint32_t) and then
there is a separate function to print, instead of the callback doing
the printing.  This will make coalescing of entries -- next commit --
easier to implement.
---
 info/nbdinfo.c | 78 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 26 deletions(-)

diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 532f7d4..088d673 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -28,6 +28,7 @@
 #include <getopt.h>
 #include <limits.h>
 #include <errno.h>
+#include <assert.h>
 
 #include <libnbd.h>
 
@@ -52,6 +53,8 @@ struct export {
 DEFINE_VECTOR_TYPE (exports, struct export)
 static exports export_list = empty_vector;
 
+DEFINE_VECTOR_TYPE (uint32_vector, uint32_t)
+
 static int collect_context (void *opaque, const char *name);
 static int collect_export (void *opaque, const char *name,
                            const char *desc);
@@ -64,6 +67,7 @@ static int extent_callback (void *user_data, const char
*metacontext,
                             uint64_t offset,
                             uint32_t *entries, size_t nr_entries,
                             int *error);
+static void print_extents (uint32_vector *entries);
 
 static void __attribute__((noreturn))
 usage (FILE *fp, int exitcode)
@@ -269,7 +273,9 @@ main (int argc, char *argv[])
     fprintf (fp, "%" PRIi64 "\n", size);
   }
   else if (map) {               /* --map (!list_all) */
-    uint64_t offset, prev_offset, align, max_len;
+    uint32_vector entries = empty_vector;
+    uint64_t offset, align, max_len;
+    size_t prev_entries_size;
 
     /* Did we get the requested map? */
     if (!nbd_can_meta_context (nbd, map)) {
@@ -287,26 +293,28 @@ main (int argc, char *argv[])
       exit (EXIT_FAILURE);
     }
 
-    if (json_output) fprintf (fp, "[\n");
     for (offset = 0; offset < size;) {
-      prev_offset = offset;
+      prev_entries_size = entries.size;
       if (nbd_block_status (nbd, MIN (size - offset, max_len), offset,
                             (nbd_extent_callback) { .callback =
extent_callback,
-                                                    .user_data = &offset },
+                                                    .user_data = &entries
},
                             0) == -1) {
         fprintf (stderr, "%s: %s\n", progname, nbd_get_error ());
         exit (EXIT_FAILURE);
       }
-      /* We expect extent_callback to increment the offset.  If it did
-       * not then probably the server is not returning any extents.
-       */
-      if (offset <= prev_offset) {
+      /* We expect extent_callback to add at least one extent to entries. */
+      if (prev_entries_size == entries.size) {
         fprintf (stderr, "%s: --map: server did not return any
extents\n",
                  progname);
         exit (EXIT_FAILURE);
       }
+      assert ((entries.size & 1) == 0);
+      for (i = prev_entries_size; i < entries.size; i += 2)
+        offset += entries.ptr[i];
     }
-    if (json_output) fprintf (fp, "\n]\n");
+
+    print_extents (&entries);
+    free (entries.ptr);
   }
   else {                        /* not --size or --map */
     /* Print per-connection fields. */
@@ -741,6 +749,30 @@ get_content (struct nbd_handle *nbd, int64_t size)
 }
 
 /* Callback handling --map. */
+static int
+extent_callback (void *user_data, const char *metacontext,
+                 uint64_t offset,
+                 uint32_t *entries, size_t nr_entries,
+                 int *error)
+{
+  uint32_vector *list = user_data;
+  size_t i;
+
+  if (strcmp (metacontext, map) != 0)
+    return 0;
+
+  /* Just append the entries we got to the list.  They are printed in
+   * print_extents below.
+   */
+  for (i = 0; i < nr_entries; ++i) {
+    if (uint32_vector_append (list, entries[i]) == -1) {
+      perror ("realloc");
+      exit (EXIT_FAILURE);
+    }
+  }
+  return 0;
+}
+
 static const char *
 extent_description (const char *metacontext, uint32_t type)
 {
@@ -769,28 +801,23 @@ extent_description (const char *metacontext, uint32_t
type)
   return NULL;   /* Don't know - description field will be omitted. */
 }
 
-static int
-extent_callback (void *user_data, const char *metacontext,
-                 uint64_t offset,
-                 uint32_t *entries, size_t nr_entries,
-                 int *error)
+static void
+print_extents (uint32_vector *entries)
 {
+  uint64_t offset = 0;
   size_t i;
-  uint64_t *ret_offset = user_data;
-  static bool comma = false;
+  bool comma = false;
 
-  if (strcmp (metacontext, map) != 0)
-    return 0;
+  if (json_output) fprintf (fp, "[\n");
 
-  /* Print the entries received. */
-  for (i = 0; i < nr_entries; i += 2) {
-    const char *descr = extent_description (map, entries[i+1]);
+  for (i = 0; i < entries->size; i += 2) {
+    const char *descr = extent_description (map, entries->ptr[i+1]);
 
     if (!json_output) {
       fprintf (fp, "%10" PRIu64 "  "
                "%10" PRIu32 "  "
                "%3" PRIu32,
-               offset, entries[i], entries[i+1]);
+               offset, entries->ptr[i], entries->ptr[i+1]);
       if (descr)
         fprintf (fp, "  %s", descr);
       fprintf (fp, "\n");
@@ -802,7 +829,7 @@ extent_callback (void *user_data, const char *metacontext,
       fprintf (fp, "{ \"offset\": %" PRIu64 ", "
                "\"length\": %" PRIu32 ", "
                "\"type\": %" PRIu32,
-               offset, entries[i], entries[i+1]);
+               offset, entries->ptr[i], entries->ptr[i+1]);
       if (descr) {
         fprintf (fp, ", \"description\": ");
         print_json_string (descr);
@@ -811,9 +838,8 @@ extent_callback (void *user_data, const char *metacontext,
       comma = true;
     }
 
-    offset += entries[i];
+    offset += entries->ptr[i];
   }
 
-  *ret_offset = offset;
-  return 0;
+  if (json_output) fprintf (fp, "\n]\n");
 }
-- 
2.29.0.rc2

Either if we are querying an extent larger than ~ 4G, or more simply
if the server returns adjacent extents of the same type, nbdinfo --map
will now coalesce them which makes the output more readable.

This also adjust an existing test and adds new tests.

I didn't feel there was any need to add a "no-coalesce" option
(although one certainly could be added) for a couple of reasons:

(1) nbdinfo is splitting some large requests itself.

(2) If you really want this information you can use the libnbd API.
---
 info/Makefile.am                       |  2 +
 info/info-map-base-allocation-large.sh |  7 ++-
 info/info-map-base-allocation-weird.sh | 49 +++++++++++++++
 info/info-map-base-allocation-zero.sh  | 37 +++++++++++
 info/nbdinfo.c                         | 85 ++++++++++++++++++--------
 5 files changed, 150 insertions(+), 30 deletions(-)

diff --git a/info/Makefile.am b/info/Makefile.am
index 8303658..c3ab780 100644
--- a/info/Makefile.am
+++ b/info/Makefile.am
@@ -32,6 +32,8 @@ info_sh_files = \
 	info-map-base-allocation.sh \
 	info-map-base-allocation-json.sh \
 	info-map-base-allocation-large.sh \
+	info-map-base-allocation-weird.sh \
+	info-map-base-allocation-zero.sh \
 	info-map-qemu-dirty-bitmap.sh \
 	info-atomic-output.sh \
 	$(NULL)
diff --git a/info/info-map-base-allocation-large.sh
b/info/info-map-base-allocation-large.sh
index 373a974..7d87368 100755
--- a/info/info-map-base-allocation-large.sh
+++ b/info/info-map-base-allocation-large.sh
@@ -30,7 +30,7 @@ rm -f $out
 
 # The sparse allocator used by nbdkit-data-plugin uses a 32K page
 # size, and extents are always aligned with this.
-nbdkit -U - data data='1 @131072 2' size=6G \
+nbdkit -U - data data='1 @131072 2 @6442450944 3' size=8G \
        --run '$VG nbdinfo --map "$uri"' > $out
 
 cat $out
@@ -38,8 +38,9 @@ cat $out
 if [ "$(tr -s ' ' < $out)" != " 0 32768 0 allocated
  32768 98304 3 hole,zero
  131072 32768 0 allocated
- 163840 4294803456 3 hole,zero
-4294967296 2147483648 3 hole,zero" ]; then
+ 163840 6442287104 3 hole,zero
+6442450944 32768 0 allocated
+6442483712 2147450880 3 hole,zero" ]; then
     echo "$0: unexpected output from nbdinfo --map"
     exit 1
 fi
diff --git a/info/info-map-base-allocation-weird.sh
b/info/info-map-base-allocation-weird.sh
new file mode 100755
index 0000000..c766ebf
--- /dev/null
+++ b/info/info-map-base-allocation-weird.sh
@@ -0,0 +1,49 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright (C) 2020 Red Hat Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. ../tests/functions.sh
+
+set -e
+set -x
+
+requires nbdkit --version
+requires nbdkit sh --version
+requires tr --version
+
+out=info-base-allocation-weird.out
+cleanup_fn rm -f $out
+rm -f $out
+
+# This is a "weird" server that returns extents that are all 1 byte.
+nbdkit -U - sh - \
+       --run '$VG nbdinfo --map "$uri"' > $out
<<'EOF'
+case "$1" in
+  get_size) echo 32 ;;
+  pread) dd if=/dev/zero count=$3 iflag=count_bytes ;;
+  can_extents) exit 0 ;;
+  extents) echo $4 1 ;;
+  *) exit 2 ;;
+esac
+EOF
+
+cat $out
+
+if [ "$(tr -s ' ' < $out)" != " 0 32 0
allocated" ]; then
+    echo "$0: unexpected output from nbdinfo --map"
+    exit 1
+fi
diff --git a/info/info-map-base-allocation-zero.sh
b/info/info-map-base-allocation-zero.sh
new file mode 100755
index 0000000..3d3d44e
--- /dev/null
+++ b/info/info-map-base-allocation-zero.sh
@@ -0,0 +1,37 @@
+#!/usr/bin/env bash
+# nbd client library in userspace
+# Copyright (C) 2020 Red Hat Inc.
+#
+# This library is free software; you can redistribute it and/or
+# modify it under the terms of the GNU Lesser General Public
+# License as published by the Free Software Foundation; either
+# version 2 of the License, or (at your option) any later version.
+#
+# This library is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+# Lesser General Public License for more details.
+#
+# You should have received a copy of the GNU Lesser General Public
+# License along with this library; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+
+. ../tests/functions.sh
+
+set -e
+set -x
+
+requires nbdkit --version
+
+out=info-base-allocation-zero.out
+cleanup_fn rm -f $out
+rm -f $out
+
+nbdkit -U - null --run '$VG nbdinfo --map "$uri"' > $out
+
+cat $out
+
+if [ "$(cat $out)" != "" ]; then
+    echo "$0: unexpected output from nbdinfo --map"
+    exit 1
+fi
diff --git a/info/nbdinfo.c b/info/nbdinfo.c
index 088d673..2a08a13 100644
--- a/info/nbdinfo.c
+++ b/info/nbdinfo.c
@@ -801,45 +801,76 @@ extent_description (const char *metacontext, uint32_t
type)
   return NULL;   /* Don't know - description field will be omitted. */
 }
 
+static void print_one_extent (uint64_t offset, uint64_t len, uint32_t type);
+
 static void
 print_extents (uint32_vector *entries)
 {
-  uint64_t offset = 0;
-  size_t i;
-  bool comma = false;
+  size_t i, j;
+  uint64_t offset = 0;          /* end of last extent printed + 1 */
+  size_t last = 0;              /* last entry printed + 2 */
 
   if (json_output) fprintf (fp, "[\n");
 
   for (i = 0; i < entries->size; i += 2) {
-    const char *descr = extent_description (map, entries->ptr[i+1]);
+    uint32_t type = entries->ptr[last+1];
 
-    if (!json_output) {
-      fprintf (fp, "%10" PRIu64 "  "
-               "%10" PRIu32 "  "
-               "%3" PRIu32,
-               offset, entries->ptr[i], entries->ptr[i+1]);
-      if (descr)
-        fprintf (fp, "  %s", descr);
-      fprintf (fp, "\n");
-    }
-    else {
-      if (comma)
-        fprintf (fp, ",\n");
+    /* If we're coalescing and the current type is different from the
+     * previous one then we should print everything up to this entry.
+     */
+    if (last != i && entries->ptr[i+1] != type) {
+      uint64_t len;
 
-      fprintf (fp, "{ \"offset\": %" PRIu64 ", "
-               "\"length\": %" PRIu32 ", "
-               "\"type\": %" PRIu32,
-               offset, entries->ptr[i], entries->ptr[i+1]);
-      if (descr) {
-        fprintf (fp, ", \"description\": ");
-        print_json_string (descr);
-      }
-      fprintf (fp, "}");
-      comma = true;
+      /* Calculate the length of the coalesced extent. */
+      for (j = last, len = 0; j < i; j += 2)
+        len += entries->ptr[j];
+      print_one_extent (offset, len, type);
+      offset += len;
+      last = i;
     }
+  }
+
+  /* Print the last extent if there is one. */
+  if (last != i) {
+    uint32_t type = entries->ptr[last+1];
+    uint64_t len;
 
-    offset += entries->ptr[i];
+    for (j = last, len = 0; j < i; j += 2)
+      len += entries->ptr[j];
+    print_one_extent (offset, len, type);
   }
 
   if (json_output) fprintf (fp, "\n]\n");
 }
+
+static void
+print_one_extent (uint64_t offset, uint64_t len, uint32_t type)
+{
+  static bool comma = false;
+  const char *descr = extent_description (map, type);
+
+  if (!json_output) {
+    fprintf (fp, "%10" PRIu64 "  "
+             "%10" PRIu64 "  "
+             "%3" PRIu32,
+             offset, len, type);
+    if (descr)
+      fprintf (fp, "  %s", descr);
+    fprintf (fp, "\n");
+  }
+  else {
+    if (comma)
+      fprintf (fp, ",\n");
+
+    fprintf (fp, "{ \"offset\": %" PRIu64 ", "
+             "\"length\": %" PRIu64 ", "
+             "\"type\": %" PRIu32,
+             offset, len, type);
+    if (descr) {
+      fprintf (fp, ", \"description\": ");
+      print_json_string (descr);
+    }
+    fprintf (fp, "}");
+    comma = true;
+  }
+}
-- 
2.29.0.rc2

On 10/27/20 1:38 PM, Richard W.M. Jones wrote:
> This library proved useful in nbdkit where we need to construct an
> array or vector of arbitrary objects, with the easy ability to append
> at the end.  Wherever code uses realloc(3) to build an array of
> objects is a candidate for replacement by this library.
> ---
>  Makefile.am              |   1 +
>  common/utils/Makefile.am |  44 ++++++++++
>  common/utils/vector.c    |  51 +++++++++++
>  common/utils/vector.h    | 177 +++++++++++++++++++++++++++++++++++++++
>  configure.ac             |   1 +
>  5 files changed, 274 insertions(+)
> 
> +++ b/common/utils/Makefile.am
> @@ -0,0 +1,44 @@
> +# nbdkit
> +# Copyright (C) 2019 Red Hat Inc.
> +#
> +# Redistribution and use in source and binary forms, with or without
> +# modification, are permitted provided that the following conditions are
> +# met:
Since we are lifting straight from nbdkit, this is fine (our one-way
license conversion at play).  It does mean we have a risk in the
opposite direction (any improvement made here can't easily be pushed
back to nbdkit unless the author is okay with the difference in license).

As long as we are lifting this, should we also list automatic cleanup
usage, where we then demand modern gcc/clang as compiler?

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