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 Fri, 2023-12-22 at 18:02 +0800, David Gow wrote:
> 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:

I think the checkpatch warning is a false positive. It seems to confuse
the * as a multiplication due to typeof() looking like a function call
rather than a variable declaration.

Benjamin

> 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
> > .





[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