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

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).) Remove the generic "name##_iter" function definition, and introduce "name##_empty", which performs both steps at the same time. Convert the call sites. (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.) Expose the "name##_empty" function definition with a new, separate macro: DEFINE_VECTOR_EMPTY(). 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. Signed-off-by: Laszlo Ersek <lersek(a)redhat.com&gt; --- common/utils/string-vector.h | 1 + common/utils/vector.h | 27 +++++++++++++------- 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 +-- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/common/utils/string-vector.h b/common/utils/string-vector.h index aa33fd48ceb5..78d0b4f9bf10 100644 --- a/common/utils/string-vector.h +++ b/common/utils/string-vector.h @@ -38,5 +38,6 @@ #include "vector.h" DEFINE_VECTOR_TYPE (string_vector, char *); +DEFINE_VECTOR_EMPTY (string_vector); #endif /* STRING_VECTOR_H */ diff --git a/common/utils/vector.h b/common/utils/vector.h index fb2482c853ff..14bf5b0ddbc0 100644 --- a/common/utils/vector.h +++ b/common/utils/vector.h @@ -150,15 +150,6 @@ v->len = v->cap = 0; \ } \ \ - /* Iterate over the vector, calling f() on each element. */ \ - static inline void __attribute__ ((__unused__)) \ - name##_iter (name *v, void (*f) (type elem)) \ - { \ - size_t i; \ - for (i = 0; i < v->len; ++i) \ - f (v->ptr[i]); \ - } \ - \

/* Sort the elements of the vector. */ \ static inline void __attribute__ ((__unused__)) \ name##_sort (name *v, \ @@ -180,6 +171,24 @@ #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 DEFINE_VECTOR_EMPTY(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) \ + free (v->ptr[i]); \ + name##_reset (v); \ + } + struct generic_vector { void *ptr; size_t len; 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); _______________________________________________ Libguestfs mailing list Libguestfs(a)redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs