On 3/21/25 7:31 AM, Ales Musil wrote: > The aim of the structure is to reduce certain type of allocations > and help with the cache consistency. There are places in the codebase > that essentially do vectoring just raw with x2nrealloc() calls. The > second category are computations that use struct ovs_list to achieve > array like iterations. > > This is preparation with the structure testing, the real replacement > is done later on. > > Signed-off-by: Ales Musil <amu...@redhat.com> > ---
Hi Ales, This seems like a very useful addition. Maybe it would fit better in OVS libraries but we can try to adopt it first in OVN and potentially move it to OVS later on. This is not a full review, I mostly just skimmed this first patch of the series for now. But I have some questions/comments below. > v2: Rebase on top of current main. > --- > lib/automake.mk | 2 + > lib/vec.c | 195 +++++++++++++++++++++++++++ > lib/vec.h | 171 ++++++++++++++++++++++++ > tests/automake.mk | 1 + > tests/ovn.at | 14 ++ > tests/test-vector.c | 316 ++++++++++++++++++++++++++++++++++++++++++++ > 6 files changed, 699 insertions(+) > create mode 100644 lib/vec.c > create mode 100644 lib/vec.h > create mode 100644 tests/test-vector.c > > diff --git a/lib/automake.mk b/lib/automake.mk > index 25e516406..a59c722d6 100644 > --- a/lib/automake.mk > +++ b/lib/automake.mk > @@ -45,6 +45,8 @@ lib_libovn_la_SOURCES = \ > lib/lb.c \ > lib/lb.h \ > lib/stopwatch-names.h \ > + lib/vec.c \ > + lib/vec.h \ > lib/vif-plug-provider.h \ > lib/vif-plug-provider.c \ > lib/vif-plug-providers/dummy/vif-plug-dummy.c > diff --git a/lib/vec.c b/lib/vec.c > new file mode 100644 > index 000000000..97e6b6549 > --- /dev/null > +++ b/lib/vec.c > @@ -0,0 +1,195 @@ > +/* Copyright (c) 2025, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > + > +#include "vec.h" > +#include "util.h" > + > +#define BYTE_SIZE(VEC, N) ((VEC)->esize * (N)) > + > +static void vector_resize(struct vector *vec, size_t new_capacity); > + > +/* Inerts element at index, the content at the index is shifted right. > + * Returns 'false' if the index is out of bounds. Note that the element > + * is pointer to the type being stored, e.g. in case of char * > + * char ** should be passed. */ > +bool > +vector_insert(struct vector *vec, size_t index, const void *element) > +{ > + if (index > vec->len) { > + return false; > + } > + > + vector_reserve(vec, 1); > + uint8_t *dst = (uint8_t *) vec->buffer + BYTE_SIZE(vec, index); > + size_t shift_len = vec->len - index; > + memmove(dst + vec->esize, dst, BYTE_SIZE(vec, shift_len)); > + memcpy(dst, element, vec->esize); > + vec->len++; > + > + return true; > +} > + > +/* Pushes array of n elements at the end of vector. */ > +void > +vector_push_array(struct vector *vec, const void *src, size_t n) > +{ > + vector_reserve(vec, n); > + memcpy((uint8_t *) vec->buffer + BYTE_SIZE(vec, vec->len), src, > + BYTE_SIZE(vec, n)); > + vec->len = vec->len + n; > +} > + > +/* Removes element at index, the argument "element" is populated with the > + * data. The content after index is shifted left. Returns 'false' if the > index > + * is out of bounds. */ > +bool > +vector_remove(struct vector *vec, size_t index, void *element) > +{ > + if (index >= vec->len || vec->len == 0) { > + return false; > + } > + > + uint8_t *dst = (uint8_t *) vec->buffer + BYTE_SIZE(vec, index); > + size_t shift_len = vec->len - index - 1; > + > + if (element) { > + memcpy(element, dst, vec->esize); > + } > + > + memmove(dst, dst + vec->esize, BYTE_SIZE(vec, shift_len)); > + vec->len--; > + > + return true; > +} > + > +/* Removes element at index, the argument "element" is populated with the > + * data. The last element takes place of the element removed, this breaks the > + * original order of insert. Returns 'false' if the index is out of bounds. > */ > +bool > +vector_remove_fast(struct vector *vec, size_t index, void *element) Should we make it the responsibility of the user? I mean, should we define a special type of vectors that preserve insertion order? We could, for example, use an internal variable in 'struct vector' to track that and fail an assertion if an user performs a fast removal that changes order. What do you think? > +{ > + if (index >= vec->len || vec->len == 0) { > + return false; > + } > + > + uint8_t *dst = (uint8_t *) vec->buffer + BYTE_SIZE(vec, index); > + uint8_t *last = (uint8_t *) vec->buffer + BYTE_SIZE(vec, vec->len - 1); > + > + if (element) { > + memcpy(element, dst, vec->esize); > + } > + > + memcpy(dst, last, vec->esize); > + vec->len--; > + > + return true; > +} > + > +/* Removes block of elements between start (inclusive) and end (exclusive), > + * The content at end is shifted left. Returns 'false' if the index > + * is out of bounds. */ > +bool > +vector_remove_block(struct vector *vec, size_t start, size_t end) > +{ > + if (vec->len == 0) { > + return false; > + } > + > + if (start >= end) { > + return false; > + } > + > + if (start >= vec->len || end > vec->len) { > + return false; > + } > + > + uint8_t *dst = (uint8_t *) vec->buffer + BYTE_SIZE(vec, start); > + uint8_t *src = (uint8_t *) vec->buffer + BYTE_SIZE(vec, end); > + size_t shift_len = vec->len - end; > + memmove(dst, src, BYTE_SIZE(vec, shift_len)); > + vec->len = vec->len + start - end; > + > + return true; > +} > + > +/* Gets pointer to the item at index. Note that holding pointer across > inserts > + * can cause UB. */ > +void * > +vector_get_ptr(const struct vector *vec, size_t index) > +{ > + if (index >= vec->len) { > + return NULL; > + } > + > + return (uint8_t *) vec->buffer + BYTE_SIZE(vec, index); > +} > + > +/* Reallocates the vector to fit exactly the length if that's not the case. > */ > +void > +vector_shrink_to_fit(struct vector *vec) > +{ > + if (vec->len == vec->capacity) { > + return; > + } > + > + vector_resize(vec, vec->len); > +} > + > +/* Clones the vector vec into new one, the content is memcopied. */ > +struct vector > +vector_clone(struct vector *vec) > +{ > + struct vector clone = (struct vector) { > + .buffer = NULL, > + .esize = vec->esize, > + .len = vec->len, > + .capacity = vec->capacity, > + }; > + > + if (vec->len) { > + clone.buffer = xmalloc(BYTE_SIZE(vec, vec->capacity)); > + memcpy(clone.buffer, vec->buffer, BYTE_SIZE(vec, vec->len)); > + } > + > + return clone; > +} > + > +/* Reserves additional space to fit extra n items. */ > +void > +vector_reserve(struct vector *vec, size_t n) > +{ > + size_t new_len = vec->len + n; > + if (new_len <= vec->capacity) { > + return; > + } > + > + vector_resize(vec, new_len <= 2 * vec->capacity ? > + 2 * vec->capacity : > + new_len); > +} > + > +static void > +vector_resize(struct vector *vec, size_t new_capacity) > +{ > + if (!new_capacity) { > + vector_destroy(vec); > + return; > + } > + > + vec->buffer = xrealloc(vec->buffer, BYTE_SIZE(vec, new_capacity)); > + vec->capacity = new_capacity; > +} > diff --git a/lib/vec.h b/lib/vec.h > new file mode 100644 > index 000000000..1bdafae14 > --- /dev/null > +++ b/lib/vec.h > @@ -0,0 +1,171 @@ > +/* Copyright (c) 2025, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#ifndef VEC_H > +#define VEC_H > + > + > +#include <stdbool.h> > +#include <stddef.h> > +#include <stdint.h> > +#include <stdlib.h> > + > +struct vector { > + void *buffer; /* Data bytes. */ > + size_t len; /* Number of elements. */ > + size_t esize; /* Size of each element in bytes. */ > + size_t capacity; /* Element capacity. */ > +}; > + > +#define VECTOR_EMPTY_INITIALIZER(TYPE) \ > + (struct vector) { \ > + .buffer = NULL, \ > + .len = 0, \ > + .esize = sizeof(TYPE), \ > + .capacity = 0 \ > + } > + > +#define VECTOR_CAPACITY_INITIALIZER(TYPE, CAP) \ > + (struct vector) { \ > + .buffer = xmalloc(sizeof (TYPE) * (CAP)), \ > + .esize = sizeof(TYPE), \ > + .len = 0, \ > + .capacity = (CAP), \ > + } > + > +/* Note that storing the returned pointer will result in UB, as the vector > + * might change the memory location during insert. */ > +#define VECTOR_FOR_EACH_PTR(VEC, NODE) > \ > + for (INIT_MULTIVAR(NODE, 0, (VEC)->buffer, OVS_TYPEOF(*NODE)); > \ > + (ITER_VAR(NODE) - (OVS_TYPEOF(*NODE) *) (VEC)->buffer < (VEC)->len > ? \ > + (((NODE) = ITER_VAR(NODE)), 1) : > \ > + 0); > \ > + UPDATE_MULTIVAR(NODE, ITER_VAR(NODE) + 1)) > + > +#define VECTOR_FOR_EACH(VEC, NODE) > \ I think this deserves a comment too as it copies the iterator value and that might not be obvious from the name of the macro. > + for (INIT_MULTIVAR(NODE, 0, (VEC)->buffer, OVS_TYPEOF(NODE)); > \ > + (ITER_VAR(NODE) - (OVS_TYPEOF(NODE) *) (VEC)->buffer < (VEC)->len ? > \ > + (((NODE) = *ITER_VAR(NODE)), 1) : > \ > + 0); > \ > + UPDATE_MULTIVAR(NODE, ITER_VAR(NODE) + 1)) > + > +#define vector_get(VEC, INDEX, TYPE) \ This probably also deserves a comment, similar to the one above VECTOR_FOR_EACH_PTR(). > + (*(TYPE *) vector_get_ptr((VEC), (INDEX))) > + > +bool vector_insert(struct vector *vec, size_t index, const void *element); > +void vector_push_array(struct vector *vec, const void *src, size_t n); > +bool vector_remove(struct vector *vec, size_t index, void *element); > +bool vector_remove_fast(struct vector *vec, size_t index, void *element); > +bool vector_remove_block(struct vector *vec, size_t start, size_t end); > +void *vector_get_ptr(const struct vector *vec, size_t index); > +void vector_shrink_to_fit(struct vector *vec); > +struct vector vector_clone(struct vector *vec); > +void vector_reserve(struct vector *vec, size_t n); > + > +/* Pushes element into the end of the vector. */ > +static inline void > +vector_push(struct vector *vec, const void *element) > +{ > + vector_insert(vec, vec->len, element); > +} > + > +/* Pops element from the end, the argument "element" is populated > + * with the data. */ > +static inline void > +vector_pop(struct vector *vec, void *element) > +{ > + vector_remove(vec, vec->len - 1, element); > +} > + > +/* Clears the vector without deallocating the buffer. */ > +static inline void > +vector_clear(struct vector *vec) > +{ > + vec->len = 0; > +} > + > +/* Initializes the vector as empty. */ > +static inline void > +vector_init(struct vector *vec) > +{ > + vec->len = 0; > + vec->capacity = 0; > + vec->buffer = NULL; > +} > + > +/* Destroys the vector content. It doesn't free individual elements, that's > up > + * to the caller. */ > +static inline void > +vector_destroy(struct vector *vec) > +{ > + free(vec->buffer); > + vector_init(vec); > +} > + > +/* Returns the length in number of elements. */ > +static inline size_t > +vector_len(const struct vector *vec) > +{ > + return vec->len; > +} > + > +/* Returns the capacity in number of elements. */ > +static inline size_t > +vector_capacity(const struct vector *vec) > +{ > + return vec->capacity; > +} > + > +/* Return true if vector is empty. */ > +static inline bool > +vector_is_empty(const struct vector * vec) > +{ > + return vec->len == 0; > +} > + > +/* Quick sort of all elements in the vector. */ > +static inline void > +vector_qsort(struct vector *vec, int (*cmp)(const void *a, const void *b)) > +{ > + if (vec->len) { I might be wrong but I think qsort(NULL, 0, .., ...) is a no-op and valid. Can we avoid the if? > + qsort(vec->buffer, vec->len, vec->esize, cmp); Also, would qsort_r() be useful? Should we provide a vector_qsort_r() wrapper too? > + } > +} > + > +/* Returns the size of allocated space for the vector elements in bytes. */ > +static inline size_t > +vector_memory_usage(struct vector *vec) > +{ > + return vec->capacity * vec->esize; > +} > + > +/* Returns the array pointer. */ > +static inline void * > +vector_get_array(const struct vector *vec) > +{ > + return vec->buffer; > +} Looking at how this function is used in the remainder of the patchset I think we're losing some of the (little) type checking we were getting from the compiler. For example, in patch 3/9: ovn_multicast_update_sbrec(const struct ovn_multicast *mc, const struct sbrec_multicast_group *sb) { - struct sbrec_port_binding **ports = xmalloc(mc->n_ports * sizeof *ports); - for (size_t i = 0; i < mc->n_ports; i++) { - ports[i] = CONST_CAST(struct sbrec_port_binding *, mc->ports[i]->sb); + struct vector sb_ports = + VECTOR_CAPACITY_INITIALIZER(struct sbrec_port_binding *, + vector_len(&mc->ports)); + + struct ovn_port *op; + VECTOR_FOR_EACH (&mc->ports, op) { + vector_push(&sb_ports, &op->sb); } - sbrec_multicast_group_set_ports(sb, ports, mc->n_ports); - free(ports); + sbrec_multicast_group_set_ports(sb, vector_get_array(&sb_ports), + vector_len(&sb_ports)); + vector_destroy(&sb_ports); } The declaration of sbrec_multicast_group_set_ports() is: void sbrec_multicast_group_set_ports(const struct sbrec_multicast_group *, struct sbrec_port_binding **ports, size_t n_ports); If you try to pass something that's not of type 'struct sbrec_port_binding **' or 'void *' as 'ports' argument, the compiler will complain. With this new vector type, we lose that because everything gets cast to 'void *' when returned by vector_get_array(). Do you think there's a way to improve this? > + > +/* Returns the array pointer, the vector is re-initialized. It is up to > caller > + * to free the array. */ > +static inline void * > +vector_steal_array(struct vector *vec) > +{ > + void *buffer = vec->buffer; > + vector_init(vec); > + return buffer; > +} Most of the concerns from my comment above also apply here I guess. > + > +#endif /* lib/vec.h */ > diff --git a/tests/automake.mk b/tests/automake.mk > index fbcd4a848..b96932dc2 100644 > --- a/tests/automake.mk > +++ b/tests/automake.mk > @@ -284,6 +284,7 @@ tests_ovstest_SOURCES = \ > tests/test-utils.c \ > tests/test-utils.h \ > tests/test-ovn.c \ > + tests/test-vector.c \ > controller/test-lflow-cache.c \ > controller/test-lflow-conj-ids.c \ > controller/test-ofctrl-seqno.c \ > diff --git a/tests/ovn.at b/tests/ovn.at > index afde2576f..025273ae4 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -2304,6 +2304,20 @@ cp test-cases.txt expout > AT_CHECK([ovstest test-ovn parse-actions < input.txt], [0], [expout]) > AT_CLEANUP > > +AT_SETUP([Vector operations]) > +check ovstest test-vector add > + > +check ovstest test-vector remove > + > +check ovstest test-vector out-of-bounds > + > +check ovstest test-vector shrink > + > +check ovstest test-vector clone > + > +check ovstest test-vector pointers > +AT_CLEANUP > + > AT_BANNER([OVN end-to-end tests]) > > OVN_FOR_EACH_NORTHD([ > diff --git a/tests/test-vector.c b/tests/test-vector.c > new file mode 100644 > index 000000000..f2bad0c33 > --- /dev/null > +++ b/tests/test-vector.c > @@ -0,0 +1,316 @@ > +/* Copyright (c) 2025, Red Hat, Inc. > + * > + * Licensed under the Apache License, Version 2.0 (the "License"); > + * you may not use this file except in compliance with the License. > + * You may obtain a copy of the License at: > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the License for the specific language governing permissions and > + * limitations under the License. > + */ > + > +#include <config.h> > + > +#include <stdint.h> > +#include <string.h> > + > +#include "lib/vec.h" > +#include "tests/ovstest.h" > +#include "lib/ovn-util.h" > + > +static void > +test_add(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + uint32_t elements[5] = {0, 1, 2, 3, 4}; > + > + struct vector vec = VECTOR_EMPTY_INITIALIZER(uint32_t); > + ovs_assert(vector_capacity(&vec) == 0); > + ovs_assert(vector_len(&vec) == 0); > + > + vector_push(&vec, &elements[1]); > + ovs_assert(vector_capacity(&vec) == 1); > + ovs_assert(vector_len(&vec) == 1); > + ovs_assert(vector_get(&vec, 0, uint32_t) == elements[1]); > + > + vector_push(&vec, &elements[2]); > + ovs_assert(vector_capacity(&vec) == 2); > + ovs_assert(vector_len(&vec) == 2); > + ovs_assert(vector_get(&vec, 1, uint32_t) == elements[2]); > + > + vector_push(&vec, &elements[3]); > + ovs_assert(vector_capacity(&vec) == 4); > + ovs_assert(vector_len(&vec) == 3); > + ovs_assert(vector_get(&vec, 2, uint32_t) == elements[3]); > + > + vector_push(&vec, &elements[4]); > + ovs_assert(vector_capacity(&vec) == 4); > + ovs_assert(vector_len(&vec) == 4); > + ovs_assert(vector_get(&vec, 3, uint32_t) == elements[4]); > + > + ovs_assert(vector_insert(&vec, 0, &elements[0])); > + ovs_assert(vector_capacity(&vec) == 8); > + ovs_assert(vector_len(&vec) == 5); > + ovs_assert(vector_get(&vec, 0, uint32_t) == elements[0]); > + > + size_t i = 0; > + uint32_t *ptr; > + VECTOR_FOR_EACH_PTR (&vec, ptr) { > + ovs_assert(elements[i++] == *ptr); > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + i = 0; > + uint32_t num; > + VECTOR_FOR_EACH (&vec, num) { > + ovs_assert(elements[i++] == num); > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + vector_destroy(&vec); > + > + vector_push_array(&vec, elements, 4); > + ovs_assert(vector_capacity(&vec) == 4); > + ovs_assert(vector_len(&vec) == 4); > + > + i = 0; > + VECTOR_FOR_EACH (&vec, num) { > + ovs_assert(elements[i++] == num); > + } > + ovs_assert(i == 4); > + > + vector_push(&vec, &elements[4]); > + ovs_assert(vector_capacity(&vec) == 8); > + ovs_assert(vector_len(&vec) == 5); > + > + i = 0; > + VECTOR_FOR_EACH (&vec, num) { > + ovs_assert(elements[i++] == num); > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + vector_destroy(&vec); > +} > + > +static void > +test_remove(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + uint32_t elements[3] = {0, 1, 2}; > + struct vector vec = VECTOR_EMPTY_INITIALIZER(uint32_t); > + vector_push_array(&vec, elements, ARRAY_SIZE(elements)); > + > + uint32_t element; > + vector_pop(&vec, &element); > + ovs_assert(vector_len(&vec) == 2); > + ovs_assert(element == 2); > + > + ovs_assert(vector_remove(&vec, 0, &element)); > + ovs_assert(vector_len(&vec) == 1); > + ovs_assert(element == 0); > + ovs_assert(vector_get(&vec, 0, uint32_t) == 1); > + > + size_t j = 0; > + uint32_t num; > + VECTOR_FOR_EACH (&vec, num) { > + ovs_assert(num == 1); > + j++; > + } > + ovs_assert(j == 1); > + > + ovs_assert(vector_remove(&vec, 0, &element)); > + ovs_assert(vector_len(&vec) == 0); > + ovs_assert(element == 1); > + > + vector_push_array(&vec, elements, ARRAY_SIZE(elements)); > + > + ovs_assert(vector_remove_fast(&vec, 0, &element)); > + ovs_assert(vector_len(&vec) == 2); > + ovs_assert(element == 0); > + > + uint32_t out_of_order[2] = {2, 1}; > + j = 0; > + VECTOR_FOR_EACH (&vec, num) { > + ovs_assert(out_of_order[j] == num); > + j++; > + } > + ovs_assert(j == ARRAY_SIZE(out_of_order)); > + > + vector_destroy(&vec); > + > + uint32_t elements2[5] = {0, 1, 2, 3, 4}; > + vector_push_array(&vec, elements2, ARRAY_SIZE(elements2)); > + ovs_assert(!vector_remove_block(&vec, 1, 1)); > + > + ovs_assert(vector_remove_block(&vec, 1, 3)); > + ovs_assert(vector_len(&vec) == 3); > + > + uint32_t block1[3] = {0, 3 ,4}; > + j = 0; > + VECTOR_FOR_EACH (&vec, num) { > + ovs_assert(block1[j] == num); > + j++; > + } > + ovs_assert(j == ARRAY_SIZE(block1)); > + > + ovs_assert(vector_remove_block(&vec, 0, 2)); > + ovs_assert(vector_len(&vec) == 1); > + ovs_assert(vector_get(&vec, 0, uint32_t) == 4); > + > + vector_destroy(&vec); > +} > + > +static void > +test_out_of_bounds(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + uint32_t num = 1; > + struct vector vec = VECTOR_EMPTY_INITIALIZER(uint32_t); > + > + ovs_assert(!vector_insert(&vec, 1, &num)); > + ovs_assert(vector_capacity(&vec) == 0); > + ovs_assert(vector_len(&vec) == 0); > + > + ovs_assert(!vector_remove(&vec, 0, &num)); > + ovs_assert(vector_capacity(&vec) == 0); > + ovs_assert(vector_len(&vec) == 0); > + > + uint32_t *ptr; > + VECTOR_FOR_EACH_PTR (&vec, ptr) { > + OVS_NOT_REACHED(); > + } > + > + VECTOR_FOR_EACH (&vec, num) { > + OVS_NOT_REACHED(); > + } > + > + vector_destroy(&vec); > +} > + > +static void > +test_shrink(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + uint32_t elements[3] = {0, 1, 2}; > + struct vector vec = VECTOR_CAPACITY_INITIALIZER(uint32_t, 10); > + vector_push_array(&vec, elements, ARRAY_SIZE(elements)); > + > + ovs_assert(vector_capacity(&vec) == 10); > + ovs_assert(vector_len(&vec) == 3); > + > + vector_shrink_to_fit(&vec); > + ovs_assert(vector_capacity(&vec) == 3); > + ovs_assert(vector_len(&vec) == 3); > + > + uint32_t num; > + vector_pop(&vec, &num); > + ovs_assert(vector_capacity(&vec) == 3); > + ovs_assert(vector_len(&vec) == 2); > + > + vector_shrink_to_fit(&vec); > + ovs_assert(vector_capacity(&vec) == 2); > + ovs_assert(vector_len(&vec) == 2); > + > + vector_push(&vec, &num); > + ovs_assert(vector_capacity(&vec) == 4); > + ovs_assert(vector_len(&vec) == 3); > + > + vector_clear(&vec); > + ovs_assert(vector_capacity(&vec) == 4); > + ovs_assert(vector_len(&vec) == 0); > + > + vector_shrink_to_fit(&vec); > + ovs_assert(vector_capacity(&vec) == 0); > + ovs_assert(vector_len(&vec) == 0); > + > + vector_destroy(&vec); > +} > + > +static void > +test_clone(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + uint32_t elements[3] = {0, 1, 2}; > + struct vector vec = VECTOR_CAPACITY_INITIALIZER(uint32_t, 10); > + vector_push_array(&vec, elements, ARRAY_SIZE(elements)); > + > + struct vector clone = vector_clone(&vec); > + ovs_assert(vector_capacity(&vec) == 10); > + ovs_assert(vector_len(&vec) == 3); > + > + size_t i = 0; > + uint32_t num; > + VECTOR_FOR_EACH (&clone, num) { > + ovs_assert(elements[i++] == num); > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + vector_shrink_to_fit(&clone); > + ovs_assert(vector_capacity(&clone) == 3); > + ovs_assert(vector_len(&clone) == 3); > + ovs_assert(vector_capacity(&vec) == 10); > + ovs_assert(vector_len(&vec) == 3); > + > + vector_destroy(&vec); > + vector_destroy(&clone); > +} > + > +static void > +test_pointers(struct ovs_cmdl_context *ctx OVS_UNUSED) > +{ > + const char *elements[3] = {"a", "b", "c"}; > + struct vector vec = VECTOR_EMPTY_INITIALIZER(const char *); > + vector_push_array(&vec, elements, ARRAY_SIZE(elements)); > + > + size_t i = 0; > + const char **ptr; > + VECTOR_FOR_EACH_PTR (&vec, ptr) { > + ovs_assert(!strcmp(elements[i++], *ptr)); > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + i = 0; > + const char *str; > + VECTOR_FOR_EACH (&vec, str) { > + ovs_assert(!strcmp(elements[i++], str)); > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + vector_destroy(&vec); > + > + vec = VECTOR_EMPTY_INITIALIZER(char *); > + for (size_t j = 0; j < ARRAY_SIZE(elements); j++) { > + char *dup = xstrdup(elements[j]); > + vector_push(&vec, &dup); > + } > + > + char *string; > + i = 0; > + VECTOR_FOR_EACH (&vec, string) { > + free(string); > + i++; > + } > + ovs_assert(i == ARRAY_SIZE(elements)); > + > + vector_destroy(&vec); > +} > + > +static void > +test_vector_main(int argc OVS_UNUSED, char *argv[] OVS_UNUSED) > +{ > + ovn_set_program_name(argv[0]); > + static const struct ovs_cmdl_command commands[] = { > + {"add", NULL, 0, 0, test_add, OVS_RO}, > + {"remove", NULL, 0, 0, test_remove, OVS_RO}, > + {"out-of-bounds", NULL, 0, 0, test_out_of_bounds, OVS_RO}, > + {"shrink", NULL, 0, 0, test_shrink, OVS_RO}, > + {"clone", NULL, 0, 0, test_clone, OVS_RO}, > + {"pointers", NULL, 0, 0, test_pointers, OVS_RO}, > + {NULL, NULL, 0, 0, NULL, OVS_RO}, > + }; > + struct ovs_cmdl_context ctx; > + ctx.argc = argc - 1; > + ctx.argv = argv + 1; > + ovs_cmdl_run_command(&ctx, commands); > +} > + > +OVSTEST_REGISTER("test-vector", test_vector_main); Otherwise, at a first glance, this looks like a very clean patch! Regards, Dumitru _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
