On 08/31/22 16:39, Eric Blake wrote:
> Instead of open-coding a linked list, we can utilize our vector.h
> helpers. No semantic change intended.
> ---
> lib/internal.h | 17 +++++----
> generator/states-newstyle-opt-meta-context.c | 36 ++++++++------------
> generator/states-reply-structured.c | 10 +++---
> lib/flags.c | 19 ++++-------
> lib/rw.c | 2 +-
> 5 files changed, 35 insertions(+), 49 deletions(-)
>
> diff --git a/lib/internal.h b/lib/internal.h
> index 0a61b15..d601d5d 100644
> --- a/lib/internal.h
> +++ b/lib/internal.h
> @@ -64,10 +64,15 @@
> */
> #define MAX_REQUEST_SIZE (64 * 1024 * 1024)
>
> -struct meta_context;
> struct socket;
> struct command;
>
> +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);
> +
> struct export {
> char *name;
> char *description;
> @@ -174,8 +179,8 @@ struct nbd_handle {
>
> bool structured_replies; /* If we negotiated NBD_OPT_STRUCTURED_REPLY
> */
>
> - /* Linked list of negotiated metadata contexts. */
> - struct meta_context *meta_contexts;
> + /* Vector of negotiated metadata contexts. */
> + meta_vector meta_contexts;
>
> /* The socket or a wrapper if using GnuTLS. */
> struct socket *sock;
> @@ -311,12 +316,6 @@ struct nbd_handle {
> bool tls_shut_writes; /* Used by lib/crypto.c to track disconnect.
> */
> };
>
> -struct meta_context {
> - struct meta_context *next; /* Linked list. */
> - char *name; /* Name of meta context. */
> - uint32_t context_id; /* Context ID negotiated with the server. */
> -};
> -
> struct socket_ops {
> ssize_t (*recv) (struct nbd_handle *h,
> struct socket *sock, void *buf, size_t len);
> diff --git a/generator/states-newstyle-opt-meta-context.c
> b/generator/states-newstyle-opt-meta-context.c
> index 30b9617..8f4dee2 100644
> --- a/generator/states-newstyle-opt-meta-context.c
> +++ b/generator/states-newstyle-opt-meta-context.c
> @@ -1,5 +1,5 @@
> /* nbd client library in userspace: state machine
> - * Copyright (C) 2013-2020 Red Hat Inc.
> + * Copyright (C) 2013-2022 Red Hat Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -44,7 +44,7 @@ STATE_MACHINE {
> }
> }
>
> - assert (h->meta_contexts == NULL);
> + assert (h->meta_contexts.len == 0);
>
> /* Calculate the length of the option request data. */
> len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
> */;
> @@ -176,7 +176,7 @@ STATE_MACHINE {
> uint32_t reply;
> uint32_t len;
> const size_t maxpayload = sizeof h->sbuf.or.payload.context;
> - struct meta_context *meta_context;
> + struct meta_context meta_context;
> uint32_t opt;
> int err = 0;
>
> @@ -202,33 +202,27 @@ STATE_MACHINE {
> debug (h, "skipping too large meta context");
> else {
> assert (len > sizeof h->sbuf.or.payload.context.context.context_id);
> - meta_context = malloc (sizeof *meta_context);
> - if (meta_context == NULL) {
> - set_error (errno, "malloc");
> - SET_NEXT_STATE (%.DEAD);
> - return 0;
> - }
> - meta_context->context_id =
> + meta_context.context_id =
> be32toh (h->sbuf.or.payload.context.context.context_id);
> /* String payload is not NUL-terminated. */
> - meta_context->name = strndup (h->sbuf.or.payload.context.str,
> - len - sizeof meta_context->context_id);
> - if (meta_context->name == NULL) {
> + meta_context.name = strndup (h->sbuf.or.payload.context.str,
> + len - sizeof meta_context.context_id);
> + if (meta_context.name == NULL) {
> set_error (errno, "strdup");
> SET_NEXT_STATE (%.DEAD);
> - free (meta_context);
> return 0;
> }
> debug (h, "negotiated %s with context ID %" PRIu32,
> - meta_context->name, meta_context->context_id);
> + meta_context.name, meta_context.context_id);
> if (opt == NBD_OPT_LIST_META_CONTEXT) {
> - CALL_CALLBACK (h->opt_cb.fn.context, meta_context->name);
> - free (meta_context->name);
> - free (meta_context);
> + CALL_CALLBACK (h->opt_cb.fn.context, meta_context.name);
> + free (meta_context.name);
> }
> - else {
> - meta_context->next = h->meta_contexts;
> - h->meta_contexts = meta_context;
> + else if (meta_vector_append (&h->meta_contexts, meta_context) == -1) {
> + set_error (errno, "realloc");
haha, I see what you did there! We're leaking our knowledge of vector's
internal use of "realloc" :)
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Laszlo
> + free (meta_context.name);
> + SET_NEXT_STATE (%.DEAD);
> + return 0;
> }
> }
> SET_NEXT_STATE (%PREPARE_FOR_REPLY);
> diff --git a/generator/states-reply-structured.c
> b/generator/states-reply-structured.c
> index 19d9fb2..aaf75b8 100644
> --- a/generator/states-reply-structured.c
> +++ b/generator/states-reply-structured.c
> @@ -454,18 +454,16 @@ STATE_MACHINE {
>
> /* Look up the context ID. */
> context_id = h->bs_entries[0];
> - for (meta_context = h->meta_contexts;
> - meta_context;
> - meta_context = meta_context->next)
> - if (context_id == meta_context->context_id)
> + for (i = 0; i < h->meta_contexts.len; ++i)
> + if (context_id == h->meta_contexts.ptr[i].context_id)
> break;
>
> - if (meta_context) {
> + if (i < h->meta_contexts.len) {
> /* Call the caller's extent function. */
> int error = cmd->error;
>
> if (CALL_CALLBACK (cmd->cb.fn.extent,
> - meta_context->name, cmd->offset,
> + h->meta_contexts.ptr[i].name, cmd->offset,
> &h->bs_entries[1], (length-4) / 4,
> &error) == -1)
> if (cmd->error == 0)
> diff --git a/lib/flags.c b/lib/flags.c
> index 44d61c8..87c20ee 100644
> --- a/lib/flags.c
> +++ b/lib/flags.c
> @@ -33,16 +33,13 @@
> void
> nbd_internal_reset_size_and_flags (struct nbd_handle *h)
> {
> - struct meta_context *m, *m_next;
> + size_t i;
>
> h->exportsize = 0;
> h->eflags = 0;
> - for (m = h->meta_contexts; m != NULL; m = m_next) {
> - m_next = m->next;
> - free (m->name);
> - free (m);
> - }
> - h->meta_contexts = NULL;
> + for (i = 0; i < h->meta_contexts.len; ++i)
> + free (h->meta_contexts.ptr[i].name);
> + meta_vector_reset (&h->meta_contexts);
> h->block_minimum = 0;
> h->block_preferred = 0;
> h->block_maximum = 0;
> @@ -195,7 +192,7 @@ nbd_unlocked_can_cache (struct nbd_handle *h)
> int
> nbd_unlocked_can_meta_context (struct nbd_handle *h, const char *name)
> {
> - struct meta_context *meta_context;
> + size_t i;
>
> if (h->eflags == 0) {
> set_error (EINVAL, "server has not returned export flags, "
> @@ -203,10 +200,8 @@ nbd_unlocked_can_meta_context (struct nbd_handle *h,
> const char *name)
> return -1;
> }
>
> - for (meta_context = h->meta_contexts;
> - meta_context;
> - meta_context = meta_context->next)
> - if (strcmp (meta_context->name, name) == 0)
> + for (i = 0; i < h->meta_contexts.len; ++i)
> + if (strcmp (h->meta_contexts.ptr[i].name, name) == 0)
> return 1;
> return 0;
> }
> diff --git a/lib/rw.c b/lib/rw.c
> index f32481a..81f8f35 100644
> --- a/lib/rw.c
> +++ b/lib/rw.c
> @@ -482,7 +482,7 @@ nbd_unlocked_aio_block_status (struct nbd_handle *h,
> return -1;
> }
>
> - if (h->meta_contexts == NULL) {
> + if (h->meta_contexts.len == 0) {
> set_error (ENOTSUP, "did not negotiate any metadata contexts, "
> "either you did not call nbd_add_meta_context before "
> "connecting or the server does not support it");
>
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs
