Re: [Libguestfs] [libnbd PATCH v2 01/12] internal: Use vector instead of linked list for meta_contexts

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");

+ 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");