Re: [PATCH 1/6] kunit: add parameter generation macro using description from array

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 20 Dec 2023 at 23:20, <benjamin@xxxxxxxxxxxxxxxx> wrote:
>
> From: Benjamin Berg <benjamin.berg@xxxxxxxxx>
>
> The existing KUNIT_ARRAY_PARAM macro requires a separate function to
> get the description. However, in a lot of cases the description can
> just be copied directly from the array. Add a second macro that
> avoids having to write a static function just for a single strscpy.
>
> Signed-off-by: Benjamin Berg <benjamin.berg@xxxxxxxxx>
> ---

I'm generally pretty happy with this, though note the checkpatch warning below.

There was some discussion at plumbers about expanding the
parameterised test APIs, so we may need to adjust the implementation
of this down the line, but I don't think that'll happen for a while,
so don't worry.

With the warnings fixed, this is:

Reviewed-by: David Gow <davidgow@xxxxxxxxxx>

I'm okay with this going in via the wireless tree if that's easier;
certainly there are some conflicts with the later patches in this
series and the kunit one.

Cheers,
-- David

>  Documentation/dev-tools/kunit/usage.rst | 12 ++++--------
>  include/kunit/test.h                    | 19 +++++++++++++++++++
>  2 files changed, 23 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/dev-tools/kunit/usage.rst b/Documentation/dev-tools/kunit/usage.rst
> index c27e1646ecd9..b959e5befcbe 100644
> --- a/Documentation/dev-tools/kunit/usage.rst
> +++ b/Documentation/dev-tools/kunit/usage.rst
> @@ -566,13 +566,9 @@ By reusing the same ``cases`` array from above, we can write the test as a
>                 },
>         };
>
> -       // Need a helper function to generate a name for each test case.
> -       static void case_to_desc(const struct sha1_test_case *t, char *desc)
> -       {
> -               strcpy(desc, t->str);
> -       }
> -       // Creates `sha1_gen_params()` to iterate over `cases`.
> -       KUNIT_ARRAY_PARAM(sha1, cases, case_to_desc);
> +       // Creates `sha1_gen_params()` to iterate over `cases` while using
> +       // the struct member `str` for the case description.
> +       KUNIT_ARRAY_PARAM_DESC(sha1, cases, str);
>
>         // Looks no different from a normal test.
>         static void sha1_test(struct kunit *test)
> @@ -588,7 +584,7 @@ By reusing the same ``cases`` array from above, we can write the test as a
>         }
>
>         // Instead of KUNIT_CASE, we use KUNIT_CASE_PARAM and pass in the
> -       // function declared by KUNIT_ARRAY_PARAM.
> +       // function declared by KUNIT_ARRAY_PARAM or KUNIT_ARRAY_PARAM_DESC.
>         static struct kunit_case sha1_test_cases[] = {
>                 KUNIT_CASE_PARAM(sha1_test, sha1_gen_params),
>                 {}
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 20ed9f9275c9..2dfa851e1f88 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -1514,6 +1514,25 @@ do {                                                                            \
>                 return NULL;                                                                    \
>         }
>
> +/**
> + * KUNIT_ARRAY_PARAM_DESC() - Define test parameter generator from an array.
> + * @name:  prefix for the test parameter generator function.
> + * @array: array of test parameters.
> + * @desc_member: structure member from array element to use as description
> + *
> + * Define function @name_gen_params which uses @array to generate parameters.
> + */
> +#define KUNIT_ARRAY_PARAM_DESC(name, array, desc_member)                                       \
> +       static const void *name##_gen_params(const void *prev, char *desc)                      \
> +       {                                                                                       \
> +               typeof((array)[0]) *__next = prev ? ((typeof(__next)) prev) + 1 : (array);      \

checkpatch is complaining here:
ERROR: need consistent spacing around '*' (ctx:WxV)
#71: FILE: include/kunit/test.h:1528:

+               typeof((array)[0]) *__next = prev ? ((typeof(__next))
prev) + 1 : (array);      \

> +               if (__next - (array) < ARRAY_SIZE((array))) {                                   \
> +                       strscpy(desc, __next->desc_member, KUNIT_PARAM_DESC_SIZE);              \
> +                       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>
> --
> 2.43.0
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@xxxxxxxxxxxxxxxx.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20231220151952.415232-2-benjamin%40sipsolutions.net.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux