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

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