On 3/24/25 03:08, Ales Musil via dev wrote:
Yes, this is one of those places where C makes this difficult. The problem is that it's not just the vector_get_array() that suffers from potential type-safety issues. The only truly type-safe way to implement vectors would be to have individual functions for each type. So like,On Fri, Mar 21, 2025 at 5:24 PM Dumitru Ceara <dce...@redhat.com> wrote: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.Hi Dumitru, thank you for the review.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 orimplied.+ * 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 withthe+ * data. The content after index is shifted left. Returns 'false' ifthe 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 withthe+ * data. The last element takes place of the element removed, thisbreaks the+ * original order of insert. Returns 'false' if the index is out ofbounds. */+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?Isn't this already the responsibility of the user? I mean this function explicitly states the intent, I don't think we need an extra check for breaking the order.+{ + 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 acrossinserts+ * 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 thecase. */+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 orimplied.+ * 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 thevector+ * 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.Sure I'll add a note about copying.+ 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().I'll add a note in v3.+ (*(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() with 0 len is no-op, qsort with NULL ptr is UB unfortunately as specified in C99 7.22.5: These utilities make use of a comparison function to search or sort arrays of unspecified type. Where an argument declared as size_t nmemb specifies the length of the array for a function, nmemb can have the value zero on a call to that function; the comparison function is not called, a search finds no matching element, and sorting performs no rearrangement. Pointer arguments on such a call shall still have valid values, as described in 7.1.4. I can change the if statement to check vec->buffer instead if you prefer. But it has to remain there unfortunately.+ qsort(vec->buffer, vec->len, vec->esize, cmp);Also, would qsort_r() be useful? Should we provide a vector_qsort_r() wrapper too?I was adding functions that will be used at least once in the code. However, if you feel like we should add qsort_r() wrapper I'm fine with that.+ } +} + +/* Returns the size of allocated space for the vector elements inbytes. */+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?Only thing that comes to my mind is that we could make it a macro instead and let the user provide the type to cast to. But it for some reason feels even worse to do this. I'm afraid that we don't have any language leverage that would help us with this.
int_vector_insert() double_vector_insert() nbrec_logical_router_port_vector_insert()and so on for each individual vector operation. This is actually doable, but it's not pretty. You'd essentially have to redeclare the vec.h file as:
#define VECTOR_DEFINE(TYPE, TYPENAME) \
\
struct TYPENAME##_vector { \
TYPE *elements; \
size_t len; \
size_t capacity; \
}; \
\
bool TYPENAME##_vector_insert(struct TYPENAME##_vector *vec, size_t
index, const TYPE *element) { \
etc.Then if you want to use an array of sbrec_port_bindings, you could put the following in a header
VECTOR_DEFINE(struct sbrec_port_binding, sbrec_port_binding);in order to create an entire set of functions to use for vectors of struct sbrec_port_binding. Short of this, you end up with void pointers, and loss of type safety.
I am NOT suggesting to actually do this. I think it's a bad idea for many reasons. I'm demonstrating that this is the sort of thing you would need to do in order to make type-safe vectors. I think we have to compromise and use void pointers. We do this all the time with other aggregates like hmap, shash, ovs_list, etc.
+ +/* Returns the array pointer, the vector is re-initialized. It is up tocaller+ * 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.Same here I'm afraid we can't do much about that.+ +#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 orimplied.+ * 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!Thank you!Regards, DumitruRegards, Ales _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
