From 07b32d5d225f63e2fe87f20e101dc3199e106423 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Sep 2022 11:52:49 +0200 Subject: [PATCH] glib-aux: add nm_g_array_index() macro and improve nm_g_array_index_p() macros Add nm_g_array_index() as a replacement for g_array_index(). The value of nm_g_array_index(), nm_g_array_index_p(), nm_g_array_first() and nm_g_array_last() is that they add nm_assert() checks for valid parameters. nm_g_array_{first,last}() now returns an lvalue and not a pointer. As such, they are just shorthands for nm_g_array_index() at index 0 and len-1, respectively. `nm_g_array_index_p(arr, Type, idx)` is almost the same as `&nm_g_array_index(arr, Type, idx)`. The only difference (and why the former variant exists), is that nm_g_array_index_p() allows to get a pointer one after the end. This means, this is correct and valid to do: // arr->len might be zero arr = nm_g_array_index_p(arr, Type, 0); for (i = 0; i < arr->len; i++, arr++) ... ptr = nm_g_array_index_p(arr, Type, 0); end = nm_g_array_index_p(arr, Type, arr->len); for (; ptr < end; ptr++) ... This would not be valid to do with nm_g_array_{index,first,last}(). Also fix supporting "const GArray *arr" parameter. Of course, the function casts the constness away. Technically, that matches the fact that arr->data is also not a const pointer. In practice, we might want to propagate the constness of the container to the constness of the element lookup. While doable, that is not implemented. --- src/core/nm-core-utils.c | 2 +- src/libnm-glib-aux/nm-shared-utils.h | 84 +++++++++++-------- .../tests/test-shared-general.c | 33 ++++++++ 3 files changed, 85 insertions(+), 34 deletions(-) diff --git a/src/core/nm-core-utils.c b/src/core/nm-core-utils.c index 0d325f0fe1..f1409c7d35 100644 --- a/src/core/nm-core-utils.c +++ b/src/core/nm-core-utils.c @@ -3022,7 +3022,7 @@ nmtst_utils_host_id_pop(void) &host_id_static, h, nmtst_host_id_stack->len == 0u ? nmtst_host_id_static_0 - : nm_g_array_last(nmtst_host_id_stack, HostIdData))) + : &nm_g_array_last(nmtst_host_id_stack, HostIdData))) g_assert_not_reached(); } diff --git a/src/libnm-glib-aux/nm-shared-utils.h b/src/libnm-glib-aux/nm-shared-utils.h index 892d0d3012..caf275f454 100644 --- a/src/libnm-glib-aux/nm-shared-utils.h +++ b/src/libnm-glib-aux/nm-shared-utils.h @@ -1866,43 +1866,61 @@ nm_g_array_unref(GArray *arr) g_array_unref(arr); } -#define nm_g_array_first(arr, Type) \ - ({ \ - GArray *const _arr = (arr); \ - \ - nm_assert(_arr); \ - nm_assert(sizeof(Type) == g_array_get_element_size(_arr)); \ - nm_assert(_arr->len > 0); \ - \ - &g_array_index(arr, Type, 0); \ - }) - -#define nm_g_array_last(arr, Type) \ - ({ \ - GArray *const _arr = (arr); \ - \ - nm_assert(_arr); \ - nm_assert(sizeof(Type) == g_array_get_element_size(_arr)); \ - nm_assert(_arr->len > 0); \ - \ - &g_array_index(arr, Type, _arr->len - 1u); \ - }) - /* Similar to g_array_index(). The differences are * - this does nm_assert() checks that the arguments are valid. - * - returns a pointer to the element. */ -#define nm_g_array_index_p(arr, Type, idx) \ - ({ \ - GArray *const _arr_55 = (arr); \ - const guint _idx_55 = (idx); \ - \ - nm_assert(_arr_55); \ - nm_assert(sizeof(Type) == g_array_get_element_size(_arr_55)); \ - nm_assert(_idx_55 < _arr_55->len); \ - \ - &g_array_index(_arr_55, Type, _idx_55); \ + * - returns a pointer to the element. + * - it asserts that @idx is <= arr->len. That is, it allows + * to get a pointer after the data, of course, you are not + * allowed to dereference in that case. */ +#define nm_g_array_index_p(arr, Type, idx) \ + ({ \ + const GArray *const _arr_55 = (arr); \ + const guint _idx_55 = (idx); \ + \ + nm_assert(_arr_55); \ + nm_assert(sizeof(Type) == g_array_get_element_size((GArray *) _arr_55)); \ + nm_assert(_idx_55 <= _arr_55->len); \ + \ + /* If arr->len is zero, arr->data might be NULL. The macro + * allows to access at index [arr->len] (one past the data). + * We need to take care of undefined behavior, but (NULL + 0) + * should work mostly fine for us. */ \ + ((Type *) ((gpointer) _arr_55->data)) + (_idx_55); \ }) +/* Very similar to g_array_index(). + * - nm_assert() that arguments are valid. + * - returns an lvalue to the element. + * - similar to nm_g_array_index_p(), but dereferences the pointer. + * - one difference to nm_g_array_index_p() is that it @idx MUST be + * smaller than arr->len (unlike nm_g_array_index_p() which allows + * access one element past the buffer. */ +#define nm_g_array_index(arr, Type, idx) \ + (*({ \ + const GArray *const _arr_55 = (arr); \ + const guint _idx_55 = (idx); \ + \ + nm_assert(_arr_55); \ + nm_assert(sizeof(Type) == g_array_get_element_size((GArray *) _arr_55)); \ + nm_assert(_idx_55 < _arr_55->len); \ + \ + &g_array_index((GArray *) _arr_55, Type, _idx_55); \ + })) + +#define nm_g_array_first(arr, Type) nm_g_array_index(arr, Type, 0) + +/* Same as g_array_index(arr, Type, arr->len-1). */ +#define nm_g_array_last(arr, Type) \ + (*({ \ + const GArray *const _arr = (arr); \ + \ + nm_assert(_arr); \ + nm_assert(sizeof(Type) == g_array_get_element_size((GArray *) _arr)); \ + nm_assert(_arr->len > 0); \ + \ + &g_array_index((GArray *) arr, Type, _arr->len - 1u); \ + })) + #define nm_g_array_append_new(arr, Type) \ ({ \ GArray *const _arr = (arr); \ diff --git a/src/libnm-glib-aux/tests/test-shared-general.c b/src/libnm-glib-aux/tests/test-shared-general.c index a49bbe0a20..b7fde16bf0 100644 --- a/src/libnm-glib-aux/tests/test-shared-general.c +++ b/src/libnm-glib-aux/tests/test-shared-general.c @@ -2218,6 +2218,38 @@ test_inet_utils(void) /*****************************************************************************/ +static void +test_garray(void) +{ + gs_unref_array GArray *arr = NULL; + int v; + + arr = g_array_new(FALSE, FALSE, sizeof(int)); + g_assert(nm_g_array_index_p(arr, int, 0) == (gpointer) arr->data); + + v = 1; + g_array_append_val(arr, v); + g_assert(nm_g_array_index_p(arr, int, 0) == (gpointer) arr->data); + g_assert(nm_g_array_index_p(arr, int, 1) == ((int *) ((gpointer) arr->data)) + 1); + g_assert(&nm_g_array_index(arr, int, 0) == (gpointer) arr->data); + g_assert(&nm_g_array_first(arr, int) == (gpointer) arr->data); + g_assert(&nm_g_array_last(arr, int) == (gpointer) arr->data); + g_assert(nm_g_array_index(arr, int, 0) == 1); + + v = 2; + g_array_append_val(arr, v); + g_assert(nm_g_array_index_p(arr, int, 0) == (gpointer) arr->data); + g_assert(nm_g_array_index_p(arr, int, 1) == ((int *) ((gpointer) arr->data)) + 1); + g_assert(nm_g_array_index_p(arr, int, 2) == ((int *) ((gpointer) arr->data)) + 2); + g_assert(&nm_g_array_index(arr, int, 0) == (gpointer) arr->data); + g_assert(&nm_g_array_first(arr, int) == (gpointer) arr->data); + g_assert(&nm_g_array_last(arr, int) == ((int *) ((gpointer) arr->data)) + 1); + g_assert(nm_g_array_index(arr, int, 0) == 1); + g_assert(nm_g_array_index(arr, int, 1) == 2); +} + +/*****************************************************************************/ + NMTST_DEFINE(); int @@ -2263,6 +2295,7 @@ main(int argc, char **argv) g_test_add_func("/general/test_path_simplify", test_path_simplify); g_test_add_func("/general/test_hostname_is_valid", test_hostname_is_valid); g_test_add_func("/general/test_inet_utils", test_inet_utils); + g_test_add_func("/general/test_garray", test_garray); return g_test_run(); }