Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 0/7] Enhance the vector library
- Optimize vector append - Refine vector fields names - Clean up vector usage in crypto module I'm fine with relicesing these change for for nbdkit. Nir Soffer (7): common/utils: Add vector benchmarks common/utils/vector.c: Optimize vector append common/utils/vector: Rename `alloc` to `cap` common/utils/vector: Rename `size` to `len` lib/crypto.c: Simplify vector reserve lib/crypto.c: Don't use empty vector lib/crypto.c: Remove unneeded else common/utils/Makefile.am | 2 +- common/utils/bench.h | 72 ++++++++++++++++++++ common/utils/test-vector.c | 51 +++++++++++++- common/utils/vector.c | 14 +++- common/utils/vector.h | 40 +++++------ copy/file-ops.c | 14 ++-- copy/main.c | 2 +- copy/multi-thread-copying.c | 2 +- copy/nbd-ops.c | 20 +++--- copy/synch-copying.c | 2 +- fuse/nbdfuse.c | 4 +- fuse/operations.c | 16 ++--- generator/states-connect-socket-activation.c | 2 +- generator/states-newstyle-opt-meta-context.c | 8 +-- info/list.c | 8 +-- info/map.c | 12 ++-- info/show.c | 6 +- lib/crypto.c | 18 ++--- lib/handle.c | 4 +- lib/uri.c | 8 +-- 20 files changed, 219 insertions(+), 86 deletions(-) create mode 100644 common/utils/bench.h -- 2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 1/7] common/utils: Add vector benchmarks
The generic vector reallocs on every append. Add benchmarks to measure
the cost with uint32 vector (used for copying extents) and the effect of
reserving space upfront.
The tests show that realloc is pretty efficient, but calling reserve
before the appends speeds the appends up significantly.
$ ./test-vector | grep bench
bench_reserve: 1000000 appends in 0.004159 s
bench_append: 1000000 appends in 0.014897 s
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
common/utils/Makefile.am | 2 +-
common/utils/bench.h | 72 ++++++++++++++++++++++++++++++++++++++
common/utils/test-vector.c | 49 ++++++++++++++++++++++++++
3 files changed, 122 insertions(+), 1 deletion(-)
create mode 100644 common/utils/bench.h
diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
index b273ada..4730cea 100644
--- a/common/utils/Makefile.am
+++ b/common/utils/Makefile.am
@@ -59,6 +59,6 @@ test_human_size_SOURCES = test-human-size.c human-size.c
human-size.h
test_human_size_CPPFLAGS = -I$(srcdir)
test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
-test_vector_SOURCES = test-vector.c vector.c vector.h
+test_vector_SOURCES = test-vector.c vector.c vector.h bench.h
test_vector_CPPFLAGS = -I$(srcdir)
test_vector_CFLAGS = $(WARNINGS_CFLAGS)
diff --git a/common/utils/bench.h b/common/utils/bench.h
new file mode 100644
index 0000000..496a361
--- /dev/null
+++ b/common/utils/bench.h
@@ -0,0 +1,72 @@
+/* libnbd
+ * Copyright (C) 2021 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.
+ */
+
+#ifndef LIBNBD_BENCH_H
+#define LIBNBD_BENCH_H
+
+#include <sys/time.h>
+
+#define MICROSECONDS 1000000
+
+struct bench {
+ struct timeval start, stop;
+};
+
+static inline void
+bench_start(struct bench *b)
+{
+ gettimeofday (&b->start, NULL);
+}
+
+static inline void
+bench_stop(struct bench *b)
+{
+ gettimeofday (&b->stop, NULL);
+}
+
+static inline double
+bench_sec(struct bench *b)
+{
+ struct timeval dt;
+
+ dt.tv_sec = b->stop.tv_sec - b->start.tv_sec;
+ dt.tv_usec = b->stop.tv_usec - b->start.tv_usec;
+
+ if (dt.tv_usec < 0) {
+ dt.tv_sec -= 1;
+ dt.tv_usec += MICROSECONDS;
+ }
+
+ return ((double)dt.tv_sec * MICROSECONDS + dt.tv_usec) / MICROSECONDS;
+}
+
+#endif /* LIBNBD_BENCH_H */
diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index 94b2aeb..28e882f 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -38,9 +38,13 @@
#undef NDEBUG /* Keep test strong even for nbdkit built without assertions */
#include <assert.h>
+#include "bench.h"
#include "vector.h"
+#define APPENDS 1000000
+
DEFINE_VECTOR_TYPE(int64_vector, int64_t);
+DEFINE_VECTOR_TYPE(uint32_vector, uint32_t);
DEFINE_VECTOR_TYPE(string_vector, char *);
static int
@@ -113,10 +117,55 @@ test_string_vector (void)
free (v.ptr);
}
+static void
+bench_reserve (void)
+{
+ uint32_vector v = empty_vector;
+ struct bench b;
+ int64_t usec;
+
+ bench_start(&b);
+
+ uint32_vector_reserve(&v, APPENDS);
+
+ for (uint32_t i = 0; i < APPENDS; i++) {
+ uint32_vector_append (&v, i);
+ }
+
+ bench_stop(&b);
+
+ assert (v.ptr[APPENDS - 1] == APPENDS - 1);
+ free (v.ptr);
+
+ printf ("bench_reserve: %d appends in %.6f s\n", APPENDS, bench_sec
(&b));
+}
+
+static void
+bench_append (void)
+{
+ uint32_vector v = empty_vector;
+ struct bench b;
+
+ bench_start(&b);
+
+ for (uint32_t i = 0; i < APPENDS; i++) {
+ uint32_vector_append (&v, i);
+ }
+
+ bench_stop(&b);
+
+ assert (v.ptr[APPENDS - 1] == APPENDS - 1);
+ free (v.ptr);
+
+ printf ("bench_append: %d appends in %.6f s\n", APPENDS, bench_sec
(&b));
+}
+
int
main (int argc, char *argv[])
{
test_int64_vector ();
test_string_vector ();
+ bench_reserve ();
+ bench_append ();
return 0;
}
--
2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 2/7] common/utils/vector.c: Optimize vector append
Minimize reallocs by growing the backing array by factor of 1.5.
Testing show that now append() is fast without calling reserve()
upfront, simplifying code using vector.
$ ./test-vector | grep bench
bench_reserve: 1000000 appends in 0.003997 s
bench_append: 1000000 appends in 0.003869 s
In practice, this can make "nbdinfo --map" 10 milliseconds faster when
running with image that have 500,000 extents.
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
common/utils/vector.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/common/utils/vector.c b/common/utils/vector.c
index 00cd254..7df17e1 100644
--- a/common/utils/vector.c
+++ b/common/utils/vector.c
@@ -41,11 +41,21 @@ int
generic_vector_reserve (struct generic_vector *v, size_t n, size_t itemsize)
{
void *newptr;
+ size_t reqalloc, newalloc;
- newptr = realloc (v->ptr, (n + v->alloc) * itemsize);
+ reqalloc = v->alloc + n;
+ if (reqalloc < v->alloc)
+ return -1; /* overflow */
+
+ newalloc = (v->alloc * 3 + 1) / 2;
+
+ if (newalloc < reqalloc)
+ newalloc = reqalloc;
+
+ newptr = realloc (v->ptr, newalloc * itemsize);
if (newptr == NULL)
return -1;
v->ptr = newptr;
- v->alloc += n;
+ v->alloc = newalloc;
return 0;
}
--
2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 3/7] common/utils/vector: Rename `alloc` to `cap`
The `alloc` field is the maximum number of items you can append to a
vector before it need to be resized. This may confuse users with the
size of the `ptr` array which is `alloc * itemsize`. Rename to "cap",
common term for this property in many languages (e.g C++, Rust, Go).
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
common/utils/vector.c | 16 ++++++++--------
common/utils/vector.h | 10 +++++-----
lib/crypto.c | 4 ++--
3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/common/utils/vector.c b/common/utils/vector.c
index 7df17e1..a4b43ce 100644
--- a/common/utils/vector.c
+++ b/common/utils/vector.c
@@ -41,21 +41,21 @@ int
generic_vector_reserve (struct generic_vector *v, size_t n, size_t itemsize)
{
void *newptr;
- size_t reqalloc, newalloc;
+ size_t reqcap, newcap;
- reqalloc = v->alloc + n;
- if (reqalloc < v->alloc)
+ reqcap = v->cap + n;
+ if (reqcap < v->cap)
return -1; /* overflow */
- newalloc = (v->alloc * 3 + 1) / 2;
+ newcap = (v->cap * 3 + 1) / 2;
- if (newalloc < reqalloc)
- newalloc = reqalloc;
+ if (newcap < reqcap)
+ newcap = reqcap;
- newptr = realloc (v->ptr, newalloc * itemsize);
+ newptr = realloc (v->ptr, newcap * itemsize);
if (newptr == NULL)
return -1;
v->ptr = newptr;
- v->alloc = newalloc;
+ v->cap = newcap;
return 0;
}
diff --git a/common/utils/vector.h b/common/utils/vector.h
index 69aad8d..3fc8b68 100644
--- a/common/utils/vector.h
+++ b/common/utils/vector.h
@@ -86,7 +86,7 @@
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. */ \
+ size_t cap; /* Maximum number of items. */ \
}; \
typedef struct name name; \
\
@@ -106,7 +106,7 @@
name##_insert (name *v, type elem, size_t i) \
{ \
assert (i <= v->size); \
- if (v->size >= v->alloc) {
\
+ if (v->size >= v->cap) {
\
if (name##_reserve (v, 1) == -1) return -1; \
} \
memmove (&v->ptr[i+1], &v->ptr[i], (v->size-i) * sizeof
(elem)); \
@@ -137,7 +137,7 @@
{ \
free (v->ptr); \
v->ptr = NULL; \
- v->size = v->alloc = 0; \
+ v->size = v->cap = 0; \
} \
\
/* Iterate over the vector, calling f() on each element. */ \
@@ -168,12 +168,12 @@
(void *) compare); \
}
-#define empty_vector { .ptr = NULL, .size = 0, .alloc = 0 }
+#define empty_vector { .ptr = NULL, .size = 0, .cap = 0 }
struct generic_vector {
void *ptr;
size_t size;
- size_t alloc;
+ size_t cap;
};
extern int generic_vector_reserve (struct generic_vector *v,
diff --git a/lib/crypto.c b/lib/crypto.c
index a9b3789..f570db3 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -141,7 +141,7 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
}
for (;;) {
- if (getlogin_r (str.ptr, str.alloc) == 0) {
+ if (getlogin_r (str.ptr, str.cap) == 0) {
return str.ptr;
}
else if (errno != ERANGE) {
@@ -150,7 +150,7 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
return NULL;
}
/* Try again with a larger buffer. */
- if (string_reserve (&str, str.alloc == 0 ? 16 : str.alloc * 2) == -1) {
+ if (string_reserve (&str, str.cap == 0 ? 16 : str.cap * 2) == -1) {
set_error (errno, "realloc");
free (str.ptr);
return NULL;
--
2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 4/7] common/utils/vector: Rename `size` to `len`
The field `size` may be confusing with the size of the underlying array.
Rename to `len`, a common term for this concept.
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
common/utils/test-vector.c | 2 +-
common/utils/vector.h | 38 ++++++++++----------
copy/file-ops.c | 14 ++++----
copy/main.c | 2 +-
copy/multi-thread-copying.c | 2 +-
copy/nbd-ops.c | 20 +++++------
copy/synch-copying.c | 2 +-
fuse/nbdfuse.c | 4 +--
fuse/operations.c | 16 ++++-----
generator/states-connect-socket-activation.c | 2 +-
generator/states-newstyle-opt-meta-context.c | 8 ++---
info/list.c | 8 ++---
info/map.c | 12 +++----
info/show.c | 6 ++--
lib/handle.c | 4 +--
lib/uri.c | 8 ++---
16 files changed, 74 insertions(+), 74 deletions(-)
diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
index 28e882f..ce9f1ce 100644
--- a/common/utils/test-vector.c
+++ b/common/utils/test-vector.c
@@ -73,7 +73,7 @@ test_int64_vector (void)
assert (v.ptr[i] == i);
int64_vector_remove (&v, 1);
- assert (v.size == 9);
+ assert (v.len == 9);
assert (v.ptr[1] == 2);
tmp = 10;
diff --git a/common/utils/vector.h b/common/utils/vector.h
index 3fc8b68..9ac0867 100644
--- a/common/utils/vector.h
+++ b/common/utils/vector.h
@@ -84,9 +84,9 @@
*/
#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 cap; /* Maximum number of items. */ \
+ type *ptr; /* Pointer to array of items. */ \
+ size_t len; /* Number of valid items in the array. */ \
+ size_t cap; /* Maximum number of items. */ \
}; \
typedef struct name name; \
\
@@ -101,17 +101,17 @@
sizeof (type)); \
} \
\
- /* Insert at i'th element. i=0 => beginning i=size => append */
\
+ /* Insert at i'th element. i=0 => beginning i=len => append */
\
static inline int \
name##_insert (name *v, type elem, size_t i) \
{ \
- assert (i <= v->size); \
- if (v->size >= v->cap) {
\
+ assert (i <= v->len); \
+ if (v->len >= v->cap) {
\
if (name##_reserve (v, 1) == -1) return -1; \
} \
- memmove (&v->ptr[i+1], &v->ptr[i], (v->size-i) * sizeof
(elem)); \
+ memmove (&v->ptr[i+1], &v->ptr[i], (v->len-i) * sizeof
(elem)); \
v->ptr[i] = elem; \
- v->size++; \
+ v->len++; \
return 0; \
} \
\
@@ -119,16 +119,16 @@
static inline int \
name##_append (name *v, type elem) \
{ \
- return name##_insert (v, elem, v->size); \
+ return name##_insert (v, elem, v->len); \
} \
\
- /* Remove i'th element. i=0 => beginning i=size-1 => end */
\
+ /* Remove i'th element. i=0 => beginning i=len-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-1) * sizeof
(type)); \
- v->size--; \
+ assert (i < v->len); \
+ memmove (&v->ptr[i], &v->ptr[i+1], (v->len-i-1) * sizeof
(type)); \
+ v->len--; \
} \
\
/* Remove all elements and deallocate the vector. */ \
@@ -137,7 +137,7 @@
{ \
free (v->ptr); \
v->ptr = NULL; \
- v->size = v->cap = 0; \
+ v->len = v->cap = 0; \
} \
\
/* Iterate over the vector, calling f() on each element. */ \
@@ -145,7 +145,7 @@
name##_iter (name *v, void (*f) (type elem)) \
{ \
size_t i; \
- for (i = 0; i < v->size; ++i) \
+ for (i = 0; i < v->len; ++i) \
f (v->ptr[i]); \
} \
\
@@ -154,7 +154,7 @@
name##_sort (name *v, \
int (*compare) (const type *p1, const type *p2)) \
{ \
- qsort (v->ptr, v->size, sizeof (type), (void *) compare); \
+ qsort (v->ptr, v->len, sizeof (type), (void *) compare); \
} \
\
/* Search for an exactly matching element in the vector using a \
@@ -164,15 +164,15 @@
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), \
+ return bsearch (key, v->ptr, v->len, sizeof (type), \
(void *) compare); \
}
-#define empty_vector { .ptr = NULL, .size = 0, .cap = 0 }
+#define empty_vector { .ptr = NULL, .len = 0, .cap = 0 }
struct generic_vector {
void *ptr;
- size_t size;
+ size_t len;
size_t cap;
};
diff --git a/copy/file-ops.c b/copy/file-ops.c
index cb787ad..8470434 100644
--- a/copy/file-ops.c
+++ b/copy/file-ops.c
@@ -101,7 +101,7 @@ page_size_init (void)
/* Load the page cache map for a particular file into
* rwf->cached_pages. Only used when reading files. This doesn't
- * fail: if a system call fails then rwf->cached_pages.size will be
+ * fail: if a system call fails then rwf->cached_pages.len will be
* zero which is handled in page_cache_evict.
*/
static inline void
@@ -121,7 +121,7 @@ page_cache_map (struct rw_file *rwf)
if (mincore (ptr, rwf->rw.size, rwf->cached_pages.ptr) == -1)
goto out;
- rwf->cached_pages.size = veclen;
+ rwf->cached_pages.len = veclen;
out:
munmap (ptr, rwf->rw.size);
}
@@ -133,7 +133,7 @@ static inline bool
page_was_cached (struct rw_file *rwf, uint64_t offset)
{
uint64_t page = offset / page_size;
- assert (page < rwf->cached_pages.size);
+ assert (page < rwf->cached_pages.len);
return (rwf->cached_pages.ptr[page] & 1) != 0;
}
@@ -151,7 +151,7 @@ page_cache_evict (struct rw_file *rwf, uint64_t orig_offset,
size_t orig_len)
* that pages were mapped so we will not evict them: essentially fall
* back to doing nothing.
*/
- if (rwf->cached_pages.size == 0) return;
+ if (rwf->cached_pages.len == 0) return;
/* Only bother with whole pages. */
offset = ROUND_UP (orig_offset, page_size);
@@ -635,7 +635,7 @@ file_get_extents (struct rw *rw, uintptr_t index,
uint64_t offset, uint64_t count,
extent_list *ret)
{
- ret->size = 0;
+ ret->len = 0;
#ifdef SEEK_HOLE
struct rw_file *rwf = (struct rw_file *)rw;
@@ -704,8 +704,8 @@ file_get_extents (struct rw *rw, uintptr_t index,
/* The last extent may extend beyond the request bounds. We must
* truncate it.
*/
- assert (ret->size > 0);
- last = ret->size - 1;
+ assert (ret->len > 0);
+ last = ret->len - 1;
assert (ret->ptr[last].offset <= end);
if (ret->ptr[last].offset + ret->ptr[last].length > end) {
uint64_t d = ret->ptr[last].offset + ret->ptr[last].length - end;
diff --git a/copy/main.c b/copy/main.c
index 15a6454..67788b5 100644
--- a/copy/main.c
+++ b/copy/main.c
@@ -526,7 +526,7 @@ default_get_extents (struct rw *rw, uintptr_t index,
{
struct extent e;
- ret->size = 0;
+ ret->len = 0;
e.offset = offset;
e.length = count;
diff --git a/copy/multi-thread-copying.c b/copy/multi-thread-copying.c
index 4603d8f..b17ca59 100644
--- a/copy/multi-thread-copying.c
+++ b/copy/multi-thread-copying.c
@@ -154,7 +154,7 @@ worker_thread (void *indexp)
else
default_get_extents (src, index, offset, count, &exts);
- for (i = 0; i < exts.size; ++i) {
+ for (i = 0; i < exts.len; ++i) {
struct command *command;
struct buffer *buffer;
char *data;
diff --git a/copy/nbd-ops.c b/copy/nbd-ops.c
index cb11e64..dfc62f2 100644
--- a/copy/nbd-ops.c
+++ b/copy/nbd-ops.c
@@ -90,7 +90,7 @@ open_one_nbd_handle (struct rw_nbd *rwn)
/* Cache these. We assume with multi-conn that each handle will act
* the same way.
*/
- if (rwn->handles.size == 0) {
+ if (rwn->handles.len == 0) {
rwn->can_zero = nbd_can_zero (nbd) > 0;
rwn->rw.size = nbd_get_size (nbd);
if (rwn->rw.size == -1) {
@@ -157,7 +157,7 @@ nbd_ops_close (struct rw *rw)
struct rw_nbd *rwn = (struct rw_nbd *) rw;
size_t i;
- for (i = 0; i < rwn->handles.size; ++i) {
+ for (i = 0; i < rwn->handles.len; ++i) {
if (nbd_shutdown (rwn->handles.ptr[i], 0) == -1) {
fprintf (stderr, "%s: %s\n", rw->name, nbd_get_error ());
exit (EXIT_FAILURE);
@@ -176,7 +176,7 @@ nbd_ops_flush (struct rw *rw)
struct rw_nbd *rwn = (struct rw_nbd *) rw;
size_t i;
- for (i = 0; i < rwn->handles.size; ++i) {
+ for (i = 0; i < rwn->handles.len; ++i) {
if (nbd_flush (rwn->handles.ptr[i], 0) == -1) {
fprintf (stderr, "%s: %s\n", rw->name, nbd_get_error ());
exit (EXIT_FAILURE);
@@ -189,7 +189,7 @@ nbd_ops_is_read_only (struct rw *rw)
{
struct rw_nbd *rwn = (struct rw_nbd *) rw;
- if (rwn->handles.size > 0)
+ if (rwn->handles.len > 0)
return nbd_is_read_only (rwn->handles.ptr[0]);
else
return false;
@@ -200,7 +200,7 @@ nbd_ops_can_extents (struct rw *rw)
{
struct rw_nbd *rwn = (struct rw_nbd *) rw;
- if (rwn->handles.size > 0)
+ if (rwn->handles.len > 0)
return nbd_can_meta_context (rwn->handles.ptr[0],
"base:allocation");
else
return false;
@@ -211,7 +211,7 @@ nbd_ops_can_multi_conn (struct rw *rw)
{
struct rw_nbd *rwn = (struct rw_nbd *) rw;
- if (rwn->handles.size > 0)
+ if (rwn->handles.len > 0)
return nbd_can_multi_conn (rwn->handles.ptr[0]);
else
return false;
@@ -226,7 +226,7 @@ nbd_ops_start_multi_conn (struct rw *rw)
for (i = 1; i < connections; ++i)
open_one_nbd_handle (rwn);
- assert (rwn->handles.size == connections);
+ assert (rwn->handles.len == connections);
}
static size_t
@@ -436,13 +436,13 @@ nbd_ops_get_extents (struct rw *rw, uintptr_t index,
nbd = rwn->handles.ptr[index];
- ret->size = 0;
+ ret->len = 0;
while (count > 0) {
const uint64_t old_offset = offset;
size_t i;
- exts.size = 0;
+ exts.len = 0;
if (nbd_block_status (nbd, count, offset,
(nbd_extent_callback) {
.user_data = &exts,
@@ -456,7 +456,7 @@ nbd_ops_get_extents (struct rw *rw, uintptr_t index,
}
/* Copy the extents returned into the final list (ret). */
- for (i = 0; i < exts.size; ++i) {
+ for (i = 0; i < exts.len; ++i) {
uint64_t d;
assert (exts.ptr[i].offset == offset);
diff --git a/copy/synch-copying.c b/copy/synch-copying.c
index c63bd2d..c9899c3 100644
--- a/copy/synch-copying.c
+++ b/copy/synch-copying.c
@@ -70,7 +70,7 @@ synch_copying (void)
else
default_get_extents (src, 0, offset, count, &exts);
- for (i = 0; i < exts.size; ++i) {
+ for (i = 0; i < exts.len; ++i) {
assert (exts.ptr[i].length <= count);
if (exts.ptr[i].zero) {
diff --git a/fuse/nbdfuse.c b/fuse/nbdfuse.c
index b82ce3e..b9d54ab 100644
--- a/fuse/nbdfuse.c
+++ b/fuse/nbdfuse.c
@@ -415,7 +415,7 @@ main (int argc, char *argv[])
handles_append (&nbd, h); /* reserved above, so can't fail */
}
}
- connections = (unsigned) nbd.size;
+ connections = (unsigned) nbd.len;
if (verbose)
fprintf (stderr, "nbdfuse: connections=%u\n", connections);
@@ -511,7 +511,7 @@ main (int argc, char *argv[])
fuse_destroy (fuse);
/* Close NBD handle(s). */
- for (i = 0; i < nbd.size; ++i)
+ for (i = 0; i < nbd.len; ++i)
nbd_close (nbd.ptr[i]);
free (mountpoint);
diff --git a/fuse/operations.c b/fuse/operations.c
index 4c14f04..264acb5 100644
--- a/fuse/operations.c
+++ b/fuse/operations.c
@@ -115,20 +115,20 @@ start_operations_threads (void)
/* This barrier is used to ensure all operations threads have
* started up before we leave this function.
*/
- err = pthread_barrier_init (&barrier, NULL, nbd.size + 1);
+ err = pthread_barrier_init (&barrier, NULL, nbd.len + 1);
if (err != 0) {
errno = err;
perror ("nbdfuse: pthread_barrier_init");
exit (EXIT_FAILURE);
}
- threads = calloc (nbd.size, sizeof (struct thread));
+ threads = calloc (nbd.len, sizeof (struct thread));
if (threads == NULL) {
perror ("calloc");
exit (EXIT_FAILURE);
}
- for (i = 0; i < nbd.size; ++i) {
+ for (i = 0; i < nbd.len; ++i) {
threads[i].n = i;
threads[i].in_flight = 0;
if ((err = pthread_mutex_init (&threads[i].in_flight_mutex, NULL)) != 0
||
@@ -234,11 +234,11 @@ next_thread (void)
{
static _Atomic size_t n = 0;
- if (nbd.size == 1)
+ if (nbd.len == 1)
return 0;
else {
size_t i = n++;
- return i % (nbd.size - 1);
+ return i % (nbd.len - 1);
}
}
@@ -509,15 +509,15 @@ nbdfuse_destroy (void *data)
*/
time (&st);
while (time (NULL) - st <= RELEASE_TIMEOUT) {
- for (i = 0; i < nbd.size; ++i) {
+ for (i = 0; i < nbd.len; ++i) {
if (threads[i].in_flight > 0)
break;
}
- if (i == nbd.size) /* no commands in flight */
+ if (i == nbd.len) /* no commands in flight */
break;
/* Signal to the operations thread to work. */
- for (i = 0; i < nbd.size; ++i) {
+ for (i = 0; i < nbd.len; ++i) {
pthread_mutex_lock (&threads[i].in_flight_mutex);
pthread_cond_signal (&threads[i].in_flight_cond);
pthread_mutex_unlock (&threads[i].in_flight_mutex);
diff --git a/generator/states-connect-socket-activation.c
b/generator/states-connect-socket-activation.c
index 8a2add3..800d963 100644
--- a/generator/states-connect-socket-activation.c
+++ b/generator/states-connect-socket-activation.c
@@ -51,7 +51,7 @@ prepare_socket_activation_environment (string_vector *env)
char *p;
size_t i;
- assert (env->size == 0);
+ assert (env->len == 0);
/* Reserve slots env[0] and env[1]. */
p = strdup ("LISTEN_PID=xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx");
diff --git a/generator/states-newstyle-opt-meta-context.c
b/generator/states-newstyle-opt-meta-context.c
index bbf155e..30b9617 100644
--- a/generator/states-newstyle-opt-meta-context.c
+++ b/generator/states-newstyle-opt-meta-context.c
@@ -38,7 +38,7 @@ STATE_MACHINE {
else {
assert (CALLBACK_IS_NULL (h->opt_cb.fn.context));
opt = NBD_OPT_SET_META_CONTEXT;
- if (!h->structured_replies || h->request_meta_contexts.size == 0) {
+ if (!h->structured_replies || h->request_meta_contexts.len == 0) {
SET_NEXT_STATE (%^OPT_GO.START);
return 0;
}
@@ -48,7 +48,7 @@ STATE_MACHINE {
/* Calculate the length of the option request data. */
len = 4 /* exportname len */ + strlen (h->export_name) + 4 /* nr queries
*/;
- for (i = 0; i < h->request_meta_contexts.size; ++i)
+ for (i = 0; i < h->request_meta_contexts.len; ++i)
len += 4 /* length of query */ + strlen
(h->request_meta_contexts.ptr[i]);
h->sbuf.option.version = htobe64 (NBD_NEW_VERSION);
@@ -87,7 +87,7 @@ STATE_MACHINE {
switch (send_from_wbuf (h)) {
case -1: SET_NEXT_STATE (%.DEAD); return 0;
case 0:
- h->sbuf.nrqueries = htobe32 (h->request_meta_contexts.size);
+ h->sbuf.nrqueries = htobe32 (h->request_meta_contexts.len);
h->wbuf = &h->sbuf;
h->wlen = sizeof h->sbuf.nrqueries;
h->wflags = MSG_MORE;
@@ -105,7 +105,7 @@ STATE_MACHINE {
return 0;
NEWSTYLE.OPT_META_CONTEXT.PREPARE_NEXT_QUERY:
- if (h->querynum >= h->request_meta_contexts.size) {
+ if (h->querynum >= h->request_meta_contexts.len) {
/* end of list of requested meta contexts */
SET_NEXT_STATE (%PREPARE_FOR_REPLY);
return 0;
diff --git a/info/list.c b/info/list.c
index 71a2acd..daef55d 100644
--- a/info/list.c
+++ b/info/list.c
@@ -75,7 +75,7 @@ free_exports (void)
{
size_t i;
- for (i = 0; i < export_list.size; ++i) {
+ for (i = 0; i < export_list.len; ++i) {
free (export_list.ptr[i].name);
free (export_list.ptr[i].desc);
}
@@ -88,10 +88,10 @@ list_all_exports (const char *uri)
size_t i;
bool list_okay = true;
- if (export_list.size == 0 && json_output)
+ if (export_list.len == 0 && json_output)
fprintf (fp, "\"exports\": []\n");
- for (i = 0; i < export_list.size; ++i) {
+ for (i = 0; i < export_list.len; ++i) {
const char *name = export_list.ptr[i].name;
struct nbd_handle *nbd2;
@@ -122,7 +122,7 @@ list_all_exports (const char *uri)
/* List the metadata of this export. */
if (!show_one_export (nbd2, export_list.ptr[i].desc, i == 0,
- i + 1 == export_list.size))
+ i + 1 == export_list.len))
list_okay = false;
if (probe_content) {
diff --git a/info/map.c b/info/map.c
index de7b7ab..39c5933 100644
--- a/info/map.c
+++ b/info/map.c
@@ -70,7 +70,7 @@ do_map (void)
}
for (offset = 0; offset < size;) {
- prev_entries_size = entries.size;
+ prev_entries_size = entries.len;
if (nbd_block_status (nbd, MIN (size - offset, max_len), offset,
(nbd_extent_callback) {
.callback = extent_callback,
@@ -80,13 +80,13 @@ do_map (void)
exit (EXIT_FAILURE);
}
/* We expect extent_callback to add at least one extent to entries. */
- if (prev_entries_size == entries.size) {
+ if (prev_entries_size == entries.len) {
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)
+ assert ((entries.len & 1) == 0);
+ for (i = prev_entries_size; i < entries.len; i += 2)
offset += entries.ptr[i];
}
@@ -134,7 +134,7 @@ print_extents (uint32_vector *entries)
if (json_output) fprintf (fp, "[\n");
- for (i = 0; i < entries->size; i += 2) {
+ for (i = 0; i < entries->len; i += 2) {
uint32_t type = entries->ptr[last+1];
/* If we're coalescing and the current type is different from the
@@ -227,7 +227,7 @@ print_totals (uint32_vector *entries, int64_t size)
uint64_t c = 0;
size_t i;
- for (i = 0; i < entries->size; i += 2) {
+ for (i = 0; i < entries->len; i += 2) {
uint32_t t = entries->ptr[i+1];
if (t == type)
diff --git a/info/show.c b/info/show.c
index ff241a8..389dece 100644
--- a/info/show.c
+++ b/info/show.c
@@ -131,7 +131,7 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
fprintf (fp, "\turi: %s\n", uri);
if (show_context) {
fprintf (fp, "\tcontexts:\n");
- for (i = 0; i < contexts.size; ++i)
+ for (i = 0; i < contexts.len; ++i)
fprintf (fp, "\t\t%s\n", contexts.ptr[i]);
}
if (is_rotational >= 0)
@@ -195,10 +195,10 @@ show_one_export (struct nbd_handle *nbd, const char *desc,
if (show_context) {
fprintf (fp, "\t\"contexts\": [\n");
- for (i = 0; i < contexts.size; ++i) {
+ for (i = 0; i < contexts.len; ++i) {
fprintf (fp, "\t\t");
print_json_string (contexts.ptr[i]);
- if (i+1 != contexts.size)
+ if (i+1 != contexts.len)
fputc (',', fp);
fputc ('\n', fp);
}
diff --git a/lib/handle.c b/lib/handle.c
index 67aa875..cbb37e8 100644
--- a/lib/handle.c
+++ b/lib/handle.c
@@ -328,7 +328,7 @@ 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 h->request_meta_contexts.size;
+ return h->request_meta_contexts.len;
}
char *
@@ -336,7 +336,7 @@ nbd_unlocked_get_meta_context (struct nbd_handle *h, size_t
i)
{
char *ret;
- if (i >= h->request_meta_contexts.size) {
+ if (i >= h->request_meta_contexts.len) {
set_error (EINVAL, "meta context request out of range");
return NULL;
}
diff --git a/lib/uri.c b/lib/uri.c
index 5e8dc06..a24dfe5 100644
--- a/lib/uri.c
+++ b/lib/uri.c
@@ -142,7 +142,7 @@ parse_uri_queries (const char *query_raw, uri_query_list
*list)
return 0;
error:
- for (i = 0; i < list->size; ++list) {
+ for (i = 0; i < list->len; ++list) {
free (list->ptr[i].name);
free (list->ptr[i].value);
}
@@ -254,7 +254,7 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
}
/* Parse the socket parameter. */
- for (i = 0; i < queries.size; i++) {
+ for (i = 0; i < queries.len; i++) {
if (strcmp (queries.ptr[i].name, "socket") == 0)
unixsocket = queries.ptr[i].value;
}
@@ -277,7 +277,7 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
goto cleanup;
/* Look for some tls-* parameters. */
- for (i = 0; i < queries.size; i++) {
+ for (i = 0; i < queries.len; i++) {
if (strcmp (queries.ptr[i].name, "tls-certificates") == 0) {
if (! h->uri_allow_local_file) {
set_error (EPERM,
@@ -387,7 +387,7 @@ nbd_unlocked_aio_connect_uri (struct nbd_handle *h, const
char *raw_uri)
ret = 0;
cleanup:
- for (i = 0; i < queries.size; ++i) {
+ for (i = 0; i < queries.len; ++i) {
free (queries.ptr[i].name);
free (queries.ptr[i].value);
}
--
2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 5/7] lib/crypto.c: Simplify vector reserve
We don't need to worry now about efficient growing.
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
lib/crypto.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/crypto.c b/lib/crypto.c
index f570db3..2ce4d4d 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -150,7 +150,7 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
return NULL;
}
/* Try again with a larger buffer. */
- if (string_reserve (&str, str.cap == 0 ? 16 : str.cap * 2) == -1) {
+ if (string_reserve (&str, 16) == -1) {
set_error (errno, "realloc");
free (str.ptr);
return NULL;
--
2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 6/7] lib/crypto.c: Don't use empty vector
getlogin_r() with empty buffer will always fail since the underlying
array is NULL. Reserve space before using the vector.
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
lib/crypto.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/crypto.c b/lib/crypto.c
index 2ce4d4d..09d98fd 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -141,6 +141,12 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
}
for (;;) {
+ /* Increase capacity (str.cap starts at 0) */
+ if (string_reserve (&str, 16) == -1) {
+ set_error (errno, "realloc");
+ free (str.ptr);
+ return NULL;
+ }
if (getlogin_r (str.ptr, str.cap) == 0) {
return str.ptr;
}
@@ -149,12 +155,6 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
free (str.ptr);
return NULL;
}
- /* Try again with a larger buffer. */
- if (string_reserve (&str, 16) == -1) {
- set_error (errno, "realloc");
- free (str.ptr);
- return NULL;
- }
}
}
--
2.31.1
Nir Soffer
2021-Oct-30 16:56 UTC
[Libguestfs] [PATCH libnbd 7/7] lib/crypto.c: Remove unneeded else
We return on successful if, so the else is not needed. This makes the
flow more clear.
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
---
lib/crypto.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/crypto.c b/lib/crypto.c
index 09d98fd..340a6a0 100644
--- a/lib/crypto.c
+++ b/lib/crypto.c
@@ -147,10 +147,12 @@ nbd_unlocked_get_tls_username (struct nbd_handle *h)
free (str.ptr);
return NULL;
}
+
if (getlogin_r (str.ptr, str.cap) == 0) {
return str.ptr;
}
- else if (errno != ERANGE) {
+
+ if (errno != ERANGE) {
set_error (errno, "getlogin_r");
free (str.ptr);
return NULL;
--
2.31.1