On 30.09.2023 10:58, David Gow wrote:
> On Wed, 27 Sept 2023 at 06:02, Michal Wajdeczko
> <michal.wajdeczko@xxxxxxxxx> wrote:
>>
>> The existing macro KUNIT_ARRAY_PARAM can produce parameter
>> generator function but only when we fully know the definition
>> of the array. However, there might be cases where we would like
>> to generate test params based on externaly defined array, which
>> is defined as zero-terminated array, like pci_driver.id_table.
>
> Hmm... I like the idea of this, but am a little wary of dealing with
> zero-terminated arrays in a generic fashion. Some cases (pointers,
> where we can just != NULL) are obvious,
> but we could hit inconsistencies with things like padding, as things
> like pci_driver.id_table seem to mostly be iterated over with things
> like:
> while (ids->vendor || ids->subvendor || ids->class_mask)
>
> which not only ignores the padding, but also half of the fields. So
> there may be a consistency issue there.
>
> Though I suspect it's not likely to cause issues in practice.
>
> Thoughts?
> -- David
>>
>> Add helper macro KUNIT_ZERO_ARRAY_PARAM that can work with zero
>> terminated arrays and provide example how to use it.
>>
>> $ ./tools/testing/kunit/kunit.py run \
>> --kunitconfig ./lib/kunit/.kunitconfig *.example_params*
>>
>> [ ] Starting KUnit Kernel (1/1)...
>> [ ] ============================================================
>> [ ] ========================= example =========================
>> [ ] =================== example_params_test ===================
>> [ ] [SKIPPED] example value 3
>> [ ] [PASSED] example value 2
>> [ ] [PASSED] example value 1
>> [ ] [SKIPPED] example value 0
>> [ ] =============== [PASSED] example_params_test ===============
>> [ ] =================== example_params_test ===================
>> [ ] [SKIPPED] example value 3
>> [ ] [PASSED] example value 2
>> [ ] [PASSED] example value 1
>> [ ] =============== [PASSED] example_params_test ===============
>> [ ] ===================== [PASSED] example =====================
>> [ ] ============================================================
>> [ ] Testing complete. Ran 7 tests: passed: 4, skipped: 3
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
>> Cc: David Gow <davidgow@xxxxxxxxxx>
>> Cc: Rae Moar <rmoar@xxxxxxxxxx>
>> ---
>> include/kunit/test.h | 22 ++++++++++++++++++++++
>> lib/kunit/kunit-example-test.c | 2 ++
>> 2 files changed, 24 insertions(+)
>>
>> diff --git a/include/kunit/test.h b/include/kunit/test.h
>> index 20ed9f9275c9..280113ceb6a6 100644
>> --- a/include/kunit/test.h
>> +++ b/include/kunit/test.h
>> @@ -1514,6 +1514,28 @@ do { \
>> return NULL; \
>> }
>>
>> +/**
>> + * KUNIT_ZERO_ARRAY_PARAM() - Define test parameter generator from a zero terminated array.
>> + * @name: prefix for the test parameter generator function.
>> + * @array: zero terminated array of test parameters.
>> + * @get_desc: function to convert param to description; NULL to use default
>> + *
>> + * Define function @name_gen_params which uses zero terminated @array to generate parameters.
>> + */
>> +#define KUNIT_ZERO_ARRAY_PARAM(name, array, get_desc) \
>> + static const void *name##_gen_params(const void *prev, char *desc) \
>> + { \
>> + typeof((array)[0]) *__prev = prev; \
>> + typeof(__prev) __next = __prev ? __prev + 1 : (array); \
>> + void (*__get_desc)(typeof(__next), char *) = get_desc; \
>> + for (; memchr_inv(__next, 0, sizeof(*__next)); __prev = __next++) { \
>
> Are there any places where this might interact awkwardly with padding?
> I _think_ it should be okay (variables with static lifetimes should
> have padding initialised to zero), but there could be a case I'm
> missing.
It looks that most of the existing code is relying on empty
initialization with = { 0 } or = { }, the latter known to be zero
initialized, including padding, in C23, so if we want to be pedantic we
may allow to provide a pointer to a table specific "is_end()" function,
that will detect end of the parameters array, which we could still
default to memchr_inv if custom solution is not needed:
#define KUNIT_ZERO_ARRAY_PARAM(name, array, get_desc, is_end) \
...
bool (*__is_end)(typeof(__next)) = is_end; \
for (; __is_end ? __is_end(__next) : \
!!memchr_inv(__next, 0, sizeof(*__next)); \
__prev = __next++) { \
KUNIT_ZERO_ARRAY_PARAM(example_zero, example_params_array,
example_param_get_desc, NULL);
or
static bool example_param_valid(const struct example_param *next)
{
return next->value;
}
KUNIT_ZERO_ARRAY_PARAM(example_zero, example_params_array,
example_param_get_desc, example_param_valid);
then we shouldn't miss any case
Michal
>
>
>> + if (__get_desc) \
>> + __get_desc(__next, desc); \
>> + return __next; \
>> + } \
>> + return NULL; \
>> + }
>> +
>> // TODO(dlatypov@xxxxxxxxxx): consider eventually migrating users to explicitly
>> // include resource.h themselves if they need it.
>> #include <kunit/resource.h>
>> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
>> index 6bb5c2ef6696..ad9ebcfd513e 100644
>> --- a/lib/kunit/kunit-example-test.c
>> +++ b/lib/kunit/kunit-example-test.c
>> @@ -202,6 +202,7 @@ static void example_param_get_desc(const struct example_param *p, char *desc)
>> }
>>
>> KUNIT_ARRAY_PARAM(example, example_params_array, example_param_get_desc);
>> +KUNIT_ZERO_ARRAY_PARAM(example_zero, example_params_array, example_param_get_desc);
>>
>> /*
>> * This test shows the use of params.
>> @@ -246,6 +247,7 @@ static struct kunit_case example_test_cases[] = {
>> KUNIT_CASE(example_all_expect_macros_test),
>> KUNIT_CASE(example_static_stub_test),
>> KUNIT_CASE_PARAM(example_params_test, example_gen_params),
>> + KUNIT_CASE_PARAM(example_params_test, example_zero_gen_params),
>> KUNIT_CASE_SLOW(example_slow_test),
>> {}
>> };
>> --
>> 2.25.1
>>
