Libguestfs - Feb 2023 - [libnbd PATCH v4 0/5] vector: introduce DEFINE_POINTER_VECTOR_TYPE()

This reworks the single

  [libnbd PATCH v3 05/29] vector: (mostly) factor out DEFINE_VECTOR_EMPTY

  http://mid.mail-archive.com/20230215141158.2426855-6-lersek at redhat.com

according to the review comments I received from Eric and Rich in that
subthread.

Clearly, the more closely I look at anything, the more refactoring
opportunities emerge. :/ At least, although I didn't like the amount of
work needed, I quite like the results. :)

This series too is bisectable.

Laszlo

Laszlo Ersek (5):
  common/include: add TYPE_IS_POINTER() macro
  common/include: extract STATIC_ASSERT() macro
  remove semicolon after DEFINE_VECTOR_TYPE() macro invocations
  vector: introduce DEFINE_POINTER_VECTOR_TYPE()
  convert string_vector_(iter(free) + reset()) to string_vector_empty()

 common/include/Makefile.am                   |  1 +
 common/include/checked-overflow.h            |  8 ++--
 common/include/compiler-macros.h             | 31 ++++++++++++-
 common/include/static-assert.h               | 48 ++++++++++++++++++++
 common/include/test-array-size.c             | 45 ++++++------------
 common/utils/const-string-vector.h           |  2 +-
 common/utils/nbdkit-string.h                 |  2 +-
 common/utils/string-vector.h                 |  2 +-
 common/utils/test-vector.c                   |  7 ++-
 common/utils/vector.h                        | 31 +++++++++++++
 copy/nbdcopy.h                               |  2 +-
 generator/states-connect-socket-activation.c |  9 ++--
 info/show.c                                  |  3 +-
 lib/handle.c                                 | 12 ++---
 lib/internal.h                               |  2 +-
 lib/utils.c                                  |  6 +--
 16 files changed, 145 insertions(+), 66 deletions(-)
 create mode 100644 common/include/static-assert.h


base-commit: da8887fd4d7ddc1f9cf2f4a3bc20fd51578fa565

Because the current definition of TYPE_IS_ARRAY() already contains a
negation, defining TYPE_IS_POINTER() in terms of TYPE_IS_ARRAY() would
lead to double negation, which I consider less than ideal style.
Therefore, introduce TYPE_IS_POINTER() from scratch, and rebase
TYPE_IS_ARRAY() on top of TYPE_IS_POINTER().

Suggested-by: Richard W.M. Jones <rjones at redhat.com>
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v4:
    - new patch in v4

 common/include/compiler-macros.h | 31 ++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/common/include/compiler-macros.h b/common/include/compiler-macros.h
index 7933bb87a5bf..beb3f4fe4e5c 100644
--- a/common/include/compiler-macros.h
+++ b/common/include/compiler-macros.h
@@ -50,12 +50,39 @@
 #define BUILD_BUG_UNLESS_TRUE(cond) \
   (BUILD_BUG_STRUCT_SIZE (cond) - BUILD_BUG_STRUCT_SIZE (cond))
 
-#define TYPE_IS_ARRAY(a) \
-  (!__builtin_types_compatible_p (typeof (a), typeof (&(a)[0])))
+/* Each of TYPE_IS_POINTER() and TYPE_IS_ARRAY() produces a build failure if it
+ * is invoked with an object that has neither pointer-to-object type nor array
+ * type.
+ *
+ * C99 6.5.2.1 constrains one of the operands of the subscript operator to have
+ * pointer-to-object type, and the other operand to have integer type. In the
+ * replacement text of TYPE_IS_POINTER(), we use [0] as subscript (providing
the
+ * integer operand), therefore the macro argument (p) is constrained to have
+ * pointer-to-object type.
+ *
+ * If TYPE_IS_POINTER() is invoked with a pointer that has pointer-to-object
+ * type, the constraint is directly satisfied, and TYPE_IS_POINTER() evaluates,
+ * at compile time, to 1.
+ *
+ * If TYPE_IS_POINTER() is invoked with an array, the constraint of the
+ * subscript operator is satisfied again -- because the array argument
"decays"
+ * to a pointer to the array's initial element (C99 6.3.2p3) --, and
+ * TYPE_IS_POINTER() evaluates, at compile time, to 0.
+ *
+ * If TYPE_IS_POINTER() is invoked with an argument having any other type, then
+ * the subscript operator constraint is not satisfied, and C99 5.1.1.3p1
+ * requires the emission of a diagnostic message -- the build breaks.
Therefore,
+ * TYPE_IS_ARRAY() can be defined simply as the logical negation of
+ * TYPE_IS_POINTER().
+ */
+#define TYPE_IS_POINTER(p) \
+  (__builtin_types_compatible_p (typeof (p), typeof (&(p)[0])))
+#define TYPE_IS_ARRAY(a) (!TYPE_IS_POINTER (a))
 
 #else /* __cplusplus */
 
 #define BUILD_BUG_UNLESS_TRUE(cond) 0
+#define TYPE_IS_POINTER(p) 1
 #define TYPE_IS_ARRAY(a) 1
 
 #endif /* __cplusplus */

We already have two use cases for static assertions (and soon we'll have
yet another). Namely:

- STATIC_ASSERT_UNSIGNED_INT() in "checked-overflow.h". Here, we use
our
  own trick, based on a negative-sized array typedef that's named with
  NBDKIT_UNIQUE_NAME.

- static_assert() in "test-array-size.c". This uses the C11 macro
called
  static_assert() from <assert.h>, which wraps the C11 _Static_assert().
  This is not really great: our baseline is C99, not C11 (per commit
  762f7c9e5166, "tests: Set minimum compiler to ISO C99.", 2021-04-08)
--
  which is why the same assertions are repeated in the code as normal
  runtime assert() calls, in case static_assert() is not defined.

Factor out our own STATIC_ASSERT(), from STATIC_ASSERT_UNSIGNED_INT().

Put it to use in "test-array-size.c", replacing both the runtime
assert()s
and the compile-time static_assert()s. Note that for the latter, in order
to remain consistent with STATIC_ASSERT_UNSIGNED_INT(), we no longer
provide the *complaint* that we want the compiler to emit upon assertion
failure, but an identifier that stands for the predicate that we *expect*.

When uncommenting the negative test case in "test-array-size.c", the
resultant wall of compiler diagnostics includes the following entry:
> test-array-size.c:83:39: error: size of array
> ?_array_size_macro_is_applied_to_array13? is negative
This patch will have to be ported to nbdkit. (IMO we can implement the
change in libnbd first -- the "common" subdir is meant to be common,
so no
particular precedence should be assumed.)

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v4:
    - new patch in v4

 common/include/checked-overflow.h |  8 ++--
 common/include/static-assert.h    | 48 ++++++++++++++++++++
 common/include/Makefile.am        |  1 +
 common/include/test-array-size.c  | 45 ++++++------------
 4 files changed, 67 insertions(+), 35 deletions(-)

diff --git a/common/include/checked-overflow.h
b/common/include/checked-overflow.h
index a1852adcdc7a..4ec72387b332 100644
--- a/common/include/checked-overflow.h
+++ b/common/include/checked-overflow.h
@@ -53,6 +53,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+#include "static-assert.h"
 #include "unique-name.h"
 
 /* Add "a" and "b", both of (possibly different) unsigned
integer types, and
@@ -173,11 +174,8 @@
  *
  * The expression "x" is not evaluated, unless it has variably
modified type.
  */
-#define STATIC_ASSERT_UNSIGNED_INT(x)                                          
\
-  do {                                                                         
\
-    typedef char NBDKIT_UNIQUE_NAME (_x_has_uint_type)[(typeof (x))-1 > 0 ?
1 : -1] \
-      __attribute__ ((__unused__));                                            
\
-  } while (0)
+#define STATIC_ASSERT_UNSIGNED_INT(x) \
+  STATIC_ASSERT ((typeof (x))-1 > 0, _x_has_uint_type)
 
 /* Assign the sum "a + b" to "*r", using uintmax_t modular
arithmetic.
  *
diff --git a/common/include/static-assert.h b/common/include/static-assert.h
new file mode 100644
index 000000000000..5a564f8946e5
--- /dev/null
+++ b/common/include/static-assert.h
@@ -0,0 +1,48 @@
+/* nbdkit
+ * Copyright (C) 2013-2023 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.
+ */
+
+#ifndef NBDKIT_STATIC_ASSERT_H
+#define NBDKIT_STATIC_ASSERT_H
+
+#include "unique-name.h"
+
+/* Assert "expression" at compile time. If "expression"
evaluates to zero (at
+ * compile time), produce a compiler error message that includes
+ * "expectation_id".
+ */
+#define STATIC_ASSERT(expression, expectation_id)                           \
+  do {                                                                      \
+    typedef char NBDKIT_UNIQUE_NAME (expectation_id)[(expression) ? 1 : -1] \
+      __attribute__ ((__unused__));                                         \
+  } while (0)
+
+#endif /* NBDKIT_STATIC_ASSERT_H */
diff --git a/common/include/Makefile.am b/common/include/Makefile.am
index fa2633c25ac3..8d6b2a88be13 100644
--- a/common/include/Makefile.am
+++ b/common/include/Makefile.am
@@ -28,6 +28,7 @@ EXTRA_DIST = \
 	iszero.h \
 	minmax.h \
 	rounding.h \
+	static-assert.h \
 	unique-name.h \
 	$(NULL)
 
diff --git a/common/include/test-array-size.c b/common/include/test-array-size.c
index 8b0972aaabe1..6244ec1e7d9e 100644
--- a/common/include/test-array-size.c
+++ b/common/include/test-array-size.c
@@ -38,6 +38,7 @@
 #include <assert.h>
 
 #include "array-size.h"
+#include "static-assert.h"
 
 struct st { const char *s; int i; };
 
@@ -60,42 +61,26 @@ static struct st st4_0[4] __attribute__ ((__unused__));
 int
 main (void)
 {
-  assert (ARRAY_SIZE (s0) == 0);
-  assert (ARRAY_SIZE (s1) == 1);
-  assert (ARRAY_SIZE (s3) == 3);
-  assert (ARRAY_SIZE (s4) == 4);
-  assert (ARRAY_SIZE (i0) == 0);
-  assert (ARRAY_SIZE (i1) == 1);
-  assert (ARRAY_SIZE (i3) == 3);
-  assert (ARRAY_SIZE (i4) == 4);
-  assert (ARRAY_SIZE (st0) == 0);
-  assert (ARRAY_SIZE (st1) == 1);
-  assert (ARRAY_SIZE (st3) == 3);
-  assert (ARRAY_SIZE (st4) == 4);
-  assert (ARRAY_SIZE (st4_0) == 4);
-
-#ifdef static_assert
-  static_assert (ARRAY_SIZE (s0) == 0, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (s1) == 1, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (s3) == 3, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (s4) == 4, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (i0) == 0, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (i1) == 1, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (i3) == 3, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (i4) == 4, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (st0) == 0, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (st1) == 1, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (st3) == 3, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (st4) == 4, "ARRAY_SIZE macro does not
work");
-  static_assert (ARRAY_SIZE (st4_0) == 4, "ARRAY_SIZE macro does not
work");
-#endif
+  STATIC_ASSERT (ARRAY_SIZE (s0) == 0, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (s1) == 1, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (s3) == 3, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (s4) == 4, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (i0) == 0, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (i1) == 1, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (i3) == 3, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (i4) == 4, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (st0) == 0, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (st1) == 1, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (st3) == 3, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (st4) == 4, _array_size_macro_works);
+  STATIC_ASSERT (ARRAY_SIZE (st4_0) == 4, _array_size_macro_works);
 
   /* You can uncomment this to test the negative case.  Unfortunately
    * it's difficult to automate this test.
    */
 #if 0
   int *p = i4;
-  assert (ARRAY_SIZE (p) == 4);
+  STATIC_ASSERT (ARRAY_SIZE (p) == 4, _array_size_macro_is_applied_to_array);
 #endif
 
   exit (EXIT_SUCCESS);

A semicolon after a DEFINE_VECTOR_TYPE(...) macro invocation is not
needed, nor does our documentation in "common/utils/vector.h"
prescribe
one. Furthermore, it breaks ISO C, which gcc reports with
"-Wpedantic":
> warning: ISO C does not allow extra ?;? outside of a function
> [-Wpedantic]
Remove the semicolons. (Note that we have such DEFINE_VECTOR_TYPE()
invocations too that are not followed by a semicolon, so our current usage
is inconsistently wrong.)

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v4:
    - new patch in v4

 lib/internal.h                     | 2 +-
 common/utils/const-string-vector.h | 2 +-
 common/utils/nbdkit-string.h       | 2 +-
 common/utils/string-vector.h       | 2 +-
 common/utils/test-vector.c         | 4 ++--
 copy/nbdcopy.h                     | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/lib/internal.h b/lib/internal.h
index 0b5f793011b8..b88b43ec6e6b 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -71,7 +71,7 @@ struct meta_context {
   char *name;                   /* Name of meta context. */
   uint32_t context_id;          /* Context ID negotiated with the server. */
 };
-DEFINE_VECTOR_TYPE (meta_vector, struct meta_context);
+DEFINE_VECTOR_TYPE (meta_vector, struct meta_context)
 
 struct export {
   char *name;
diff --git a/common/utils/const-string-vector.h
b/common/utils/const-string-vector.h
index c15f8952b3b3..d537fedd646f 100644
--- a/common/utils/const-string-vector.h
+++ b/common/utils/const-string-vector.h
@@ -37,6 +37,6 @@
 
 #include "vector.h"
 
-DEFINE_VECTOR_TYPE (const_string_vector, const char *);
+DEFINE_VECTOR_TYPE (const_string_vector, const char *)
 
 #endif /* CONST_STRING_VECTOR_H */
diff --git a/common/utils/nbdkit-string.h b/common/utils/nbdkit-string.h
index b85f05991a82..81bbe9f45784 100644
--- a/common/utils/nbdkit-string.h
+++ b/common/utils/nbdkit-string.h
@@ -37,6 +37,6 @@
 
 #include "vector.h"
 
-DEFINE_VECTOR_TYPE (string, char);
+DEFINE_VECTOR_TYPE (string, char)
 
 #endif /* NBDKIT_STRING_H */
diff --git a/common/utils/string-vector.h b/common/utils/string-vector.h
index aa33fd48ceb5..373eb319120a 100644
--- a/common/utils/string-vector.h
+++ b/common/utils/string-vector.h
@@ -37,6 +37,6 @@
 
 #include "vector.h"
 
-DEFINE_VECTOR_TYPE (string_vector, char *);
+DEFINE_VECTOR_TYPE (string_vector, char *)
 
 #endif /* STRING_VECTOR_H */
diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index 55c8ebeb8a1e..27c8da1c06ab 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -50,8 +50,8 @@
 
 #define APPENDS 1000000
 
-DEFINE_VECTOR_TYPE (int64_vector, int64_t);
-DEFINE_VECTOR_TYPE (uint32_vector, uint32_t);
+DEFINE_VECTOR_TYPE (int64_vector, int64_t)
+DEFINE_VECTOR_TYPE (uint32_vector, uint32_t)
 
 static int
 compare (const int64_t *a, const int64_t *b)
diff --git a/copy/nbdcopy.h b/copy/nbdcopy.h
index b6257cc13951..5e1e078b34cb 100644
--- a/copy/nbdcopy.h
+++ b/copy/nbdcopy.h
@@ -116,7 +116,7 @@ struct extent {
   uint64_t length;
   bool zero;
 };
-DEFINE_VECTOR_TYPE (extent_list, struct extent);
+DEFINE_VECTOR_TYPE (extent_list, struct extent)
 
 /* The operations struct hides some of the differences between local
  * file, NBD and pipes from the copying code.

The "name##_iter" function is used 11 times in libnbd; in all those
cases,
"name" is "string_vector", and the "f" callback is
"free":

  string_vector_iter (..., (void *) free);

Casting "free" to (void*) is ugly. (Well-defined by POSIX, but still.)

Furthermore, in all 11 cases, the freeing of the vector's strings is
immediately followed by the release of the vector's array-of-pointers too.
(This additional step is not expressed consistently across libnbd: it's
sometimes spelled as free(vec.ptr), sometimes as
string_vector_reset(&vec).)

Introduce "name##_empty", which performs both steps at the same time.

Keep the generic "name##_iter" function definition, as we'll want
to synch
this patch to nbdkit, and in nbdkit, "name##_iter" has other uses as
well.

Expose the "name##_empty" function definition with a new, separate
macro:
ADD_VECTOR_EMPTY_METHOD(). The existent DEFINE_VECTOR_TYPE() macro permits
such element types that are not pointers, or are pointers to const- and/or
volatile-qualified objects. Whereas "name##_empty" requires that the
elements be pointers to dynamically allocated, non-const, non-volatile
objects.

Add DEFINE_POINTER_VECTOR_TYPE() that expands to both DEFINE_VECTOR_TYPE()
and the additive ADD_VECTOR_EMPTY_METHOD().

(

For example, after

  typedef char foobar[5];

the following compiles (as expected):

  DEFINE_VECTOR_TYPE (foobar_vector, foobar)

and the following fails to build (as expected):

  DEFINE_POINTER_VECTOR_TYPE (foobar_vector_bad, foobar)

with the diagnostics including
> In function ?foobar_vector_bad_empty?:
> error: size of array ?_vector_contains_pointers1? is negative
)

Switch "string_vector" to DEFINE_POINTER_VECTOR_TYPE(). The 11
string_vector_iter() call sites continue working; they will be converted
to string_vector_empty() in a subsequent patch.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v4:
    
    - rename DEFINE_VECTOR_EMPTY to ADD_VECTOR_EMPTY_METHOD
    
    - introduce DEFINE_POINTER_VECTOR_TYPE as a convenience macro for
      DEFINE_VECTOR_TYPE plus ADD_VECTOR_EMPTY_METHOD
    
    - keep "name##_iter"
    
    - statically assert in "name##_empty" that the vector contains
pointers
    
    - redefine string_vector with DEFINE_POINTER_VECTOR_TYPE now, rather
      than augmenting it as DEFINE_VECTOR_TYPE + (pre-rename, additive)
      DEFINE_VECTOR_EMPTY
    
    - split the call site conversions to a separate patch

 common/utils/string-vector.h |  2 +-
 common/utils/vector.h        | 31 ++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/common/utils/string-vector.h b/common/utils/string-vector.h
index 373eb319120a..d049a8d5b2b1 100644
--- a/common/utils/string-vector.h
+++ b/common/utils/string-vector.h
@@ -37,6 +37,6 @@
 
 #include "vector.h"
 
-DEFINE_VECTOR_TYPE (string_vector, char *)
+DEFINE_POINTER_VECTOR_TYPE (string_vector, char *)
 
 #endif /* STRING_VECTOR_H */
diff --git a/common/utils/vector.h b/common/utils/vector.h
index fb2482c853ff..38272925a65a 100644
--- a/common/utils/vector.h
+++ b/common/utils/vector.h
@@ -44,6 +44,9 @@
 #include <assert.h>
 #include <string.h>
 
+#include "compiler-macros.h"
+#include "static-assert.h"
+
 #ifdef __clang__
 #pragma clang diagnostic ignored "-Wunused-function"
 #pragma clang diagnostic ignored "-Wduplicate-decl-specifier"
@@ -180,6 +183,34 @@
 
 #define empty_vector { .ptr = NULL, .len = 0, .cap = 0 }
 
+/* This macro should only be used if:
+ * - the vector contains pointers, and
+ * - the pointed-to objects are:
+ *   - neither const- nor volatile-qualified, and
+ *   - allocated with malloc() or equivalent.
+ */
+#define ADD_VECTOR_EMPTY_METHOD(name)                                  \
+  /* Call free() on each element of the vector, then reset the vector. \
+   */                                                                  \
+  static inline void __attribute__ ((__unused__))                      \
+  name##_empty (name *v)                                               \
+  {                                                                    \
+    size_t i;                                                          \
+    for (i = 0; i < v->len; ++i) {                                     \
+      STATIC_ASSERT (TYPE_IS_POINTER (v->ptr[i]),                      \
+                     _vector_contains_pointers);                       \
+      free (v->ptr[i]);                                                \
+    }                                                                  \
+    name##_reset (v);                                                  \
+  }
+
+/* Convenience macro tying together DEFINE_VECTOR_TYPE() and
+ * ADD_VECTOR_EMPTY_METHOD().
+ */
+#define DEFINE_POINTER_VECTOR_TYPE(name, type) \
+  DEFINE_VECTOR_TYPE (name, type)              \
+  ADD_VECTOR_EMPTY_METHOD (name)
+
 struct generic_vector {
   void *ptr;
   size_t len;

Convert the 11 string_vector_(iter(free) + reset()) call sites mentioned
previously to string_vector_empty().

Note that the converted code performs more cleanup steps in some cases
than strictly necessary, but the extra work is harmless, and arguably
beneficial for clarity / consistency.

Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---

Notes:
    v4:
    
    - split out from the previous [libnbd PATCH v3 05/29] vector: (mostly)
      factor out DEFINE_VECTOR_EMPTY

 generator/states-connect-socket-activation.c |  9 +++------
 lib/handle.c                                 | 12 ++++--------
 lib/utils.c                                  |  6 ++----
 common/utils/test-vector.c                   |  3 +--
 info/show.c                                  |  3 +--
 5 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/generator/states-connect-socket-activation.c
b/generator/states-connect-socket-activation.c
index 7138e7638e30..3b621b8be44f 100644
--- a/generator/states-connect-socket-activation.c
+++ b/generator/states-connect-socket-activation.c
@@ -91,8 +91,7 @@ prepare_socket_activation_environment (string_vector *env)
 
  err:
   set_error (errno, "malloc");
-  string_vector_iter (env, (void *) free);
-  free (env->ptr);
+  string_vector_empty (env);
   return -1;
 }
 
@@ -166,8 +165,7 @@  CONNECT_SA.START:
     SET_NEXT_STATE (%.DEAD);
     set_error (errno, "fork");
     close (s);
-    string_vector_iter (&env, (void *) free);
-    free (env.ptr);
+    string_vector_empty (&env);
     return 0;
   }
   if (pid == 0) {         /* child - run command */
@@ -210,8 +208,7 @@  CONNECT_SA.START:
 
   /* Parent. */
   close (s);
-  string_vector_iter (&env, (void *) free);
-  free (env.ptr);
+  string_vector_empty (&env);
   h->pid = pid;
 
   h->connaddrlen = sizeof addr;
diff --git a/lib/handle.c b/lib/handle.c
index dfd8c2e5cdb9..fb92f16e6c91 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -128,8 +128,7 @@ nbd_close (struct nbd_handle *h)
   /* Free user callbacks first. */
   nbd_unlocked_clear_debug_callback (h);
 
-  string_vector_iter (&h->querylist, (void *) free);
-  free (h->querylist.ptr);
+  string_vector_empty (&h->querylist);
   free (h->bs_entries);
   nbd_internal_reset_size_and_flags (h);
   for (i = 0; i < h->meta_contexts.len; ++i)
@@ -139,8 +138,7 @@ 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);
-  string_vector_iter (&h->argv, (void *) free);
-  free (h->argv.ptr);
+  string_vector_empty (&h->argv);
   if (h->sact_sockpath) {
     if (h->pid > 0)
       kill (h->pid, SIGTERM);
@@ -164,8 +162,7 @@ nbd_close (struct nbd_handle *h)
   free (h->tls_certificates);
   free (h->tls_username);
   free (h->tls_psk_file);
-  string_vector_iter (&h->request_meta_contexts, (void *) free);
-  free (h->request_meta_contexts.ptr);
+  string_vector_empty (&h->request_meta_contexts);
   free (h->hname);
   pthread_mutex_destroy (&h->lock);
   free (h);
@@ -379,8 +376,7 @@ nbd_unlocked_get_meta_context (struct nbd_handle *h, size_t
i)
 int
 nbd_unlocked_clear_meta_contexts (struct nbd_handle *h)
 {
-  string_vector_iter (&h->request_meta_contexts, (void *) free);
-  string_vector_reset (&h->request_meta_contexts);
+  string_vector_empty (&h->request_meta_contexts);
   return 0;
 }
 
diff --git a/lib/utils.c b/lib/utils.c
index c229ebfc6106..bba4b3846e77 100644
--- a/lib/utils.c
+++ b/lib/utils.c
@@ -93,8 +93,7 @@ nbd_internal_set_argv (struct nbd_handle *h, char **argv)
     return -1;
   }
 
-  string_vector_iter (&h->argv, (void *) free);
-  string_vector_reset (&h->argv);
+  string_vector_empty (&h->argv);
 
   if (nbd_internal_copy_string_list (&h->argv, argv) == -1) {
     set_error (errno, "realloc");
@@ -110,8 +109,7 @@ nbd_internal_set_argv (struct nbd_handle *h, char **argv)
 int
 nbd_internal_set_querylist (struct nbd_handle *h, char **queries)
 {
-  string_vector_iter (&h->querylist, (void *) free);
-  string_vector_reset (&h->querylist);
+  string_vector_empty (&h->querylist);
 
   if (queries) {
     if (nbd_internal_copy_string_list (&h->querylist, queries) == -1) {
diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index 27c8da1c06ab..8143669e95b9 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -151,8 +151,7 @@ test_string_vector (void)
     printf ("%s\n", v.ptr[i]);
   assert (i == 10);
 
-  string_vector_iter (&v, (void*)free);
-  string_vector_reset (&v);
+  string_vector_empty (&v);
 }
 
 static void
diff --git a/info/show.c b/info/show.c
index 4bf596715cf9..e6c9b9bf1243 100644
--- a/info/show.c
+++ b/info/show.c
@@ -275,8 +275,7 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
       fprintf (fp, "\t},\n");
   }
 
-  string_vector_iter (&contexts, (void *) free);
-  free (contexts.ptr);
+  string_vector_empty (&contexts);
   free (content);
   free (export_name);
   free (export_desc);

On Fri, Feb 24, 2023 at 05:39:32AM +0100, Laszlo Ersek
wrote:
> This reworks the single
> 
>   [libnbd PATCH v3 05/29] vector: (mostly) factor out DEFINE_VECTOR_EMPTY
> 
>   http://mid.mail-archive.com/20230215141158.2426855-6-lersek at redhat.com
> 
> according to the review comments I received from Eric and Rich in that
> subthread.
> 
> Clearly, the more closely I look at anything, the more refactoring
> opportunities emerge. :/ At least, although I didn't like the amount of
> work needed, I quite like the results. :)
> 
> This series too is bisectable.
> 
> Laszlo
> 
> Laszlo Ersek (5):
>   common/include: add TYPE_IS_POINTER() macro
>   common/include: extract STATIC_ASSERT() macro
>   remove semicolon after DEFINE_VECTOR_TYPE() macro invocations
>   vector: introduce DEFINE_POINTER_VECTOR_TYPE()
>   convert string_vector_(iter(free) + reset()) to string_vector_empty()
And turned out very nicely in my opinion.  See my comments on 3.  For
1,2,4,5:

Reviewed-by: Eric Blake <eblake at redhat.com>

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