Laszlo Ersek
2023-Feb-26 11:45 UTC
[Libguestfs] [libnbd PATCH v5 0/5] vector: introduce DEFINE_POINTER_VECTOR_TYPE()
V4: http://mid.mail-archive.com/20230224043937.392235-1-lersek at redhat.com Please see the Notes section in each patch for the updates. Laszlo Laszlo Ersek (5): common/include: add TYPE_IS_POINTER() macro common/include: extract STATIC_ASSERT() macro force 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/string-vector.h | 2 +- common/utils/test-vector.c | 3 +- common/utils/vector.h | 42 ++++++++++++++++- copy/file-ops.c | 2 +- copy/nbd-ops.c | 2 +- dump/dump.c | 2 +- fuse/nbdfuse.h | 2 +- generator/states-connect-socket-activation.c | 9 ++-- info/list.c | 2 +- info/map.c | 2 +- info/show.c | 3 +- lib/handle.c | 12 ++--- lib/uri.c | 2 +- lib/utils.c | 6 +-- ublk/nbdublk.h | 2 +- ublk/tgt.c | 2 +- 21 files changed, 157 insertions(+), 71 deletions(-) create mode 100644 common/include/static-assert.h base-commit: 594f7a738e0aba23eda37965c96b3df6b8f76960
Laszlo Ersek
2023-Feb-26 11:45 UTC
[Libguestfs] [libnbd PATCH v5 1/5] common/include: add TYPE_IS_POINTER() macro
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>
Reviewed-by: Eric Blake <eblake at redhat.com>
---
Notes:
v5:
- pick up Eric's R-b
- no change
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 */
Laszlo Ersek
2023-Feb-26 11:45 UTC
[Libguestfs] [libnbd PATCH v5 2/5] common/include: extract STATIC_ASSERT() macro
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 negativeThis 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> Reviewed-by: Eric Blake <eblake at redhat.com> --- Notes: v5: - pick up Eric's R-b - no change 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);
Laszlo Ersek
2023-Feb-26 11:45 UTC
[Libguestfs] [libnbd PATCH v5 3/5] force semicolon after DEFINE_VECTOR_TYPE() macro invocations
Currently, 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]Our current usage is inconsistent; a proper subset of all our DEFINE_VECTOR_TYPE() invocations is succeeded by a semicolon. We could remove these semicolons, but that doesn't play nicely with Emacs's C language parser. Thus, force the semicolon instead. Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- Notes: v5: - force semicolon after DEFINE_VECTOR_TYPE() [Eric] - reword commit message accordingly v4: - new patch in v4 common/utils/vector.h | 7 +++++-- lib/uri.c | 2 +- copy/file-ops.c | 2 +- copy/nbd-ops.c | 2 +- dump/dump.c | 2 +- fuse/nbdfuse.h | 2 +- info/list.c | 2 +- info/map.c | 2 +- ublk/nbdublk.h | 2 +- ublk/tgt.c | 2 +- 10 files changed, 14 insertions(+), 11 deletions(-) diff --git a/common/utils/vector.h b/common/utils/vector.h index fb2482c853ff..b9b88ba02e7d 100644 --- a/common/utils/vector.h +++ b/common/utils/vector.h @@ -52,7 +52,7 @@ /* 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 *) + * 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: @@ -176,7 +176,10 @@ { \ return bsearch (key, v->ptr, v->len, sizeof (type), \ (void *) compare); \ - } + } \ + \ + /* End with duplicate declaration, so callers must supply ';'. */ \ + struct name #define empty_vector { .ptr = NULL, .len = 0, .cap = 0 } diff --git a/lib/uri.c b/lib/uri.c index 367621d40208..31ee90f3b94f 100644 --- a/lib/uri.c +++ b/lib/uri.c @@ -58,7 +58,7 @@ struct uri_query { char *value; }; -DEFINE_VECTOR_TYPE (uri_query_list, struct uri_query) +DEFINE_VECTOR_TYPE (uri_query_list, struct uri_query); /* Parse the query_raw substring of a URI into a list of decoded queries. * Return 0 on success or -1 on error. diff --git a/copy/file-ops.c b/copy/file-ops.c index 18cae74a617d..1efece2614e6 100644 --- a/copy/file-ops.c +++ b/copy/file-ops.c @@ -64,7 +64,7 @@ #endif #ifdef PAGE_CACHE_MAPPING -DEFINE_VECTOR_TYPE (byte_vector, uint8_t) +DEFINE_VECTOR_TYPE (byte_vector, uint8_t); #endif static struct rw_ops file_ops; diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c index 34ab4857ee00..0980a5edec46 100644 --- a/copy/nbd-ops.c +++ b/copy/nbd-ops.c @@ -33,7 +33,7 @@ static struct rw_ops nbd_ops; -DEFINE_VECTOR_TYPE (handles, struct nbd_handle *) +DEFINE_VECTOR_TYPE (handles, struct nbd_handle *); struct rw_nbd { struct rw rw; diff --git a/dump/dump.c b/dump/dump.c index d0da28790eb7..922bd4355897 100644 --- a/dump/dump.c +++ b/dump/dump.c @@ -38,7 +38,7 @@ #include "version.h" #include "vector.h" -DEFINE_VECTOR_TYPE (uint32_vector, uint32_t) +DEFINE_VECTOR_TYPE (uint32_vector, uint32_t); static const char *progname; static struct nbd_handle *nbd; diff --git a/fuse/nbdfuse.h b/fuse/nbdfuse.h index 371ca8bb53ac..93b66aac52c1 100644 --- a/fuse/nbdfuse.h +++ b/fuse/nbdfuse.h @@ -30,7 +30,7 @@ #include "vector.h" -DEFINE_VECTOR_TYPE (handles, struct nbd_handle *) +DEFINE_VECTOR_TYPE (handles, struct nbd_handle *); extern handles nbd; extern unsigned connections; diff --git a/info/list.c b/info/list.c index c2741ba0fa10..4b4e8f860160 100644 --- a/info/list.c +++ b/info/list.c @@ -35,7 +35,7 @@ struct export { char *name; char *desc; }; -DEFINE_VECTOR_TYPE (exports, struct export) +DEFINE_VECTOR_TYPE (exports, struct export); static exports export_list = empty_vector; static int diff --git a/info/map.c b/info/map.c index a5aad9552208..4924866a3b28 100644 --- a/info/map.c +++ b/info/map.c @@ -36,7 +36,7 @@ #include "nbdinfo.h" -DEFINE_VECTOR_TYPE (uint32_vector, uint32_t) +DEFINE_VECTOR_TYPE (uint32_vector, uint32_t); static void print_extents (uint32_vector *entries); static void print_totals (uint32_vector *entries, int64_t size); diff --git a/ublk/nbdublk.h b/ublk/nbdublk.h index 086352e9d17f..1296b8424a5b 100644 --- a/ublk/nbdublk.h +++ b/ublk/nbdublk.h @@ -25,7 +25,7 @@ #include "vector.h" -DEFINE_VECTOR_TYPE (handles, struct nbd_handle *) +DEFINE_VECTOR_TYPE (handles, struct nbd_handle *); #define UBLKSRV_TGT_TYPE_NBD 0 diff --git a/ublk/tgt.c b/ublk/tgt.c index 5d88e33dcc4b..9b0a64d66b80 100644 --- a/ublk/tgt.c +++ b/ublk/tgt.c @@ -62,7 +62,7 @@ struct thread_info { struct ublksrv_aio_ctx *aio_ctx; struct ublksrv_aio_list compl; }; -DEFINE_VECTOR_TYPE (thread_infos, struct thread_info) +DEFINE_VECTOR_TYPE (thread_infos, struct thread_info); static thread_infos thread_info; static pthread_barrier_t barrier;
Laszlo Ersek
2023-Feb-26 11:45 UTC
[Libguestfs] [libnbd PATCH v5 4/5] vector: introduce DEFINE_POINTER_VECTOR_TYPE()
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: v5: - resolve conflict from update to previous patch - force semicolon after ADD_VECTOR_EMPTY_METHOD() and DEFINE_POINTER_VECTOR_TYPE() too - update example macro invocations in the commit message 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 | 35 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/common/utils/string-vector.h b/common/utils/string-vector.h index aa33fd48ceb5..115d7d484c71 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 b9b88ba02e7d..bdba02f7ebee 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" @@ -183,6 +186,38 @@ #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); \ + } \ + \ + /* Force callers to supply ';'. */ \ + struct name + +/* Convenience macro tying together DEFINE_VECTOR_TYPE() and + * ADD_VECTOR_EMPTY_METHOD(). Inherit and forward the requirement for a + * trailing semicolon from ADD_VECTOR_EMPTY_METHOD() to the caller. + */ +#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;
Laszlo Ersek
2023-Feb-26 11:45 UTC
[Libguestfs] [libnbd PATCH v5 5/5] convert string_vector_(iter(free) + reset()) to string_vector_empty()
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>
Reviewed-by: Eric Blake <eblake at redhat.com>
---
Notes:
v5:
- pick up Eric's R-b
- no change
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 55c8ebeb8a1e..05ac5cec3dba 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);
Laszlo Ersek
2023-Feb-28 11:36 UTC
[Libguestfs] [libnbd PATCH v5 0/5] vector: introduce DEFINE_POINTER_VECTOR_TYPE()
On 2/26/23 12:45, Laszlo Ersek wrote:> V4: > http://mid.mail-archive.com/20230224043937.392235-1-lersek at redhat.com > > Please see the Notes section in each patch for the updates. > > LaszloMerged as commit range 254ac22dde42..c7ff70101e8c. Thanks Laszlo> > Laszlo Ersek (5): > common/include: add TYPE_IS_POINTER() macro > common/include: extract STATIC_ASSERT() macro > force 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/string-vector.h | 2 +- > common/utils/test-vector.c | 3 +- > common/utils/vector.h | 42 ++++++++++++++++- > copy/file-ops.c | 2 +- > copy/nbd-ops.c | 2 +- > dump/dump.c | 2 +- > fuse/nbdfuse.h | 2 +- > generator/states-connect-socket-activation.c | 9 ++-- > info/list.c | 2 +- > info/map.c | 2 +- > info/show.c | 3 +- > lib/handle.c | 12 ++--- > lib/uri.c | 2 +- > lib/utils.c | 6 +-- > ublk/nbdublk.h | 2 +- > ublk/tgt.c | 2 +- > 21 files changed, 157 insertions(+), 71 deletions(-) > create mode 100644 common/include/static-assert.h > > > base-commit: 594f7a738e0aba23eda37965c96b3df6b8f76960 > _______________________________________________ > Libguestfs mailing list > Libguestfs at redhat.com > https://listman.redhat.com/mailman/listinfo/libguestfs >
Richard W.M. Jones
2023-Feb-28 12:43 UTC
[Libguestfs] [libnbd PATCH v5 0/5] vector: introduce DEFINE_POINTER_VECTOR_TYPE()
Thanks for this series. I checked it upstream & it looks great. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org