Eric Blake
2022-Aug-31 14:39 UTC
[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"); -- 2.37.2
Laszlo Ersek
2022-Sep-01 07:39 UTC
[Libguestfs] [libnbd PATCH v2 01/12] internal: Use vector instead of linked list for meta_contexts
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 <lersek at 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"); >