| Message ID | 1583251361-12748-3-git-send-email-alan.maguire@oracle.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | kunit: extend kunit resources API | expand |
On Tue, Mar 3, 2020 at 8:03 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > The kunit resources API allows for custom initialization and > cleanup code (init/fini); here we use the init code to set the > new "struct kunit_resource" "name" field, and call an additional > init function if needed. Having a simple way to name resources > is useful in cases such as multithreaded tests where a set of > resources are shared among threads; a pointer to the > "struct kunit *" test state then is all that is needed to > retrieve and use named resources. Support is provided to add, > find and destroy named resources; the latter two are simply > wrappers that use a "match-by-name" callback. > > If an attempt to add a resource with a name that already exists > is made kunit_add_named_resource() will return NULL. > > Signed-off-by: Alan Maguire <alan.maguire@oracle.com> Overall, I think this seems reasonable. I think it needs a use case to be justified, so long as Patricia ends up using it. Reviewed-by: Brendan Higgins <brendanhiggins@google.com> > --- > include/kunit/test.h | 40 ++++++++++++++++++++++++++++++++++++++- > lib/kunit/kunit-test.c | 37 ++++++++++++++++++++++++++++++++++++ > lib/kunit/test.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 127 insertions(+), 1 deletion(-) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 11c80f5..70ee581 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -73,9 +73,14 @@ > * kunit_kmalloc_free, ¶ms); > * } > * > + * Resources can also be named, with lookup/removal done on a name > + * basis also. kunit_add_named_resource(), kunit_find_named_resource() > + * and kunit_destroy_named_resource() below. Resource names must be > + * unique within the test instance. > */ > struct kunit_resource { > void *data; > + const char *name; /* optional name */ > kunit_resource_init_t init; > kunit_resource_free_t free; > > @@ -275,12 +280,27 @@ struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, > * @init: a user-supplied function to initialize the result (if needed). If > * none is supplied, the resource data value is simply set to @data. > * If an init function is supplied, @data is passed to it instead. > - * @free: a user-supplied function to free the resource (if needed). > + * @free: a user-supplied function to free the resource data (if needed). > * @data: value to pass to init function or set in resource data field. > */ > int kunit_add_resource(struct kunit *test, kunit_resource_init_t init, > kunit_resource_free_t free, struct kunit_resource *res, > void *data); > + > +/** > + * kunit_add_named_resource() - Add a named *test managed resource*. > + * @test: The test context object. > + * @init: a user-supplied function to initialize the resource data, if needed. > + * @free: a user-supplied function to free the resource data, if needed. > + * @name_data: name and data to be set for resource. > + */ > +int kunit_add_named_resource(struct kunit *test, > + kunit_resource_init_t init, > + kunit_resource_free_t free, > + struct kunit_resource *res, > + const char *name, > + void *data); > + > /** > * kunit_alloc_resource() - Allocates a *test managed resource*. > * @test: The test context object. > @@ -336,6 +356,19 @@ static inline bool kunit_resource_instance_match(struct kunit *test, > } > > /** > + * kunit_resource_name_match() - Match a resource with the same name. > + * @test: Test case to which the resource belongs. > + * @res: The resource. > + * @match_name: The name to match against. > + */ > +static inline bool kunit_resource_name_match(struct kunit *test, > + struct kunit_resource *res, > + void *match_name) > +{ > + return res->name && strcmp(res->name, match_name) == 0; > +} > + > +/** > * kunit_find_resource() - Find a resource using match function/data. > * @test: Test case to which the resource belongs. > * @match: match function to be applied to resources/match data. > @@ -345,6 +378,9 @@ struct kunit_resource *kunit_find_resource(struct kunit *test, > kunit_resource_match_t match, > void *match_data); > > +#define kunit_find_named_resource(test, name) \ > + kunit_find_resource(test, kunit_resource_name_match, (void *)name) > + > /** > * kunit_destroy_resource() - Find a kunit_resource and destroy it. > * @test: Test case to which the resource belongs. > @@ -358,6 +394,8 @@ int kunit_destroy_resource(struct kunit *test, > kunit_resource_match_t match, > void *match_data); > > +#define kunit_destroy_named_resource(test, name) \ > + kunit_destroy_resource(test, kunit_resource_name_match, name) nit: I would prefer a static inline function here instead of a macro. > /** > * kunit_remove_resource: remove resource from resource list associated with > diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c > index b8bf36d..079c558 100644 > --- a/lib/kunit/kunit-test.c > +++ b/lib/kunit/kunit-test.c > @@ -317,6 +317,42 @@ static void kunit_resource_test_static(struct kunit *test) > KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); > } > > +static void kunit_resource_test_named(struct kunit *test) > +{ > + struct kunit_resource res1, res2, *found = NULL; > + struct kunit_test_resource_context ctx; > + > + KUNIT_EXPECT_EQ(test, > + kunit_add_named_resource(test, NULL, NULL, &res1, > + "resource_1", &ctx), > + 0); > + KUNIT_EXPECT_PTR_EQ(test, res1.data, (void *)&ctx); > + > + KUNIT_EXPECT_EQ(test, > + kunit_add_named_resource(test, NULL, NULL, &res1, > + "resource_1", &ctx), > + -EEXIST); > + > + KUNIT_EXPECT_EQ(test, > + kunit_add_named_resource(test, NULL, NULL, &res2, > + "resource_2", &ctx), > + 0); > + > + found = kunit_find_named_resource(test, "resource_1"); > + > + KUNIT_EXPECT_PTR_EQ(test, found, &res1); > + > + if (found) > + kunit_put_resource(&res1); > + > + KUNIT_EXPECT_EQ(test, kunit_destroy_named_resource(test, "resource_2"), > + 0); > + > + kunit_cleanup(test); > + > + KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); > +} > + > static int kunit_resource_test_init(struct kunit *test) > { > struct kunit_test_resource_context *ctx = > @@ -346,6 +382,7 @@ static void kunit_resource_test_exit(struct kunit *test) > KUNIT_CASE(kunit_resource_test_cleanup_resources), > KUNIT_CASE(kunit_resource_test_proper_free_ordering), > KUNIT_CASE(kunit_resource_test_static), > + KUNIT_CASE(kunit_resource_test_named), > {} > }; > > diff --git a/lib/kunit/test.c b/lib/kunit/test.c > index 132e9bf..86a4d9c 100644 > --- a/lib/kunit/test.c > +++ b/lib/kunit/test.c > @@ -380,6 +380,57 @@ int kunit_add_resource(struct kunit *test, kunit_resource_init_t init, > } > EXPORT_SYMBOL_GPL(kunit_add_resource); > > +/* Used to initialize named resource. */ > +struct kunit_name_data { > + const char *name; > + kunit_resource_init_t init; > + void *data; > +}; > + > +static int kunit_init_named_resource(struct kunit_resource *res, void *data) > +{ > + struct kunit_name_data *name_data = data; > + > + res->name = name_data->name; > + res->data = name_data->data; > + res->init = name_data->init; > + > + if (res->init) > + return res->init(res, name_data->data); > + > + res->data = name_data->data; > + > + return 0; > +} > + > +int kunit_add_named_resource(struct kunit *test, > + kunit_resource_init_t init, > + kunit_resource_free_t free, > + struct kunit_resource *res, > + const char *name, > + void *data) > +{ > + struct kunit_name_data name_data; > + struct kunit_resource *existing; > + > + if (!name) > + return -EINVAL; > + > + existing = kunit_find_named_resource(test, name); > + if (existing) { > + kunit_put_resource(existing); > + return -EEXIST; > + } > + > + name_data.name = name; > + name_data.init = init; > + name_data.data = data; > + > + return kunit_add_resource(test, kunit_init_named_resource, free, res, > + &name_data); > +} > +EXPORT_SYMBOL_GPL(kunit_add_named_resource); Could we maybe combine the above with the non-named variants? Seems like there might be some unnecessary code duplication. > struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, > kunit_resource_init_t init, > kunit_resource_free_t free, Once again, thank you for all your hard work!
diff --git a/include/kunit/test.h b/include/kunit/test.h index 11c80f5..70ee581 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -73,9 +73,14 @@ * kunit_kmalloc_free, ¶ms); * } * + * Resources can also be named, with lookup/removal done on a name + * basis also. kunit_add_named_resource(), kunit_find_named_resource() + * and kunit_destroy_named_resource() below. Resource names must be + * unique within the test instance. */ struct kunit_resource { void *data; + const char *name; /* optional name */ kunit_resource_init_t init; kunit_resource_free_t free; @@ -275,12 +280,27 @@ struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, * @init: a user-supplied function to initialize the result (if needed). If * none is supplied, the resource data value is simply set to @data. * If an init function is supplied, @data is passed to it instead. - * @free: a user-supplied function to free the resource (if needed). + * @free: a user-supplied function to free the resource data (if needed). * @data: value to pass to init function or set in resource data field. */ int kunit_add_resource(struct kunit *test, kunit_resource_init_t init, kunit_resource_free_t free, struct kunit_resource *res, void *data); + +/** + * kunit_add_named_resource() - Add a named *test managed resource*. + * @test: The test context object. + * @init: a user-supplied function to initialize the resource data, if needed. + * @free: a user-supplied function to free the resource data, if needed. + * @name_data: name and data to be set for resource. + */ +int kunit_add_named_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + const char *name, + void *data); + /** * kunit_alloc_resource() - Allocates a *test managed resource*. * @test: The test context object. @@ -336,6 +356,19 @@ static inline bool kunit_resource_instance_match(struct kunit *test, } /** + * kunit_resource_name_match() - Match a resource with the same name. + * @test: Test case to which the resource belongs. + * @res: The resource. + * @match_name: The name to match against. + */ +static inline bool kunit_resource_name_match(struct kunit *test, + struct kunit_resource *res, + void *match_name) +{ + return res->name && strcmp(res->name, match_name) == 0; +} + +/** * kunit_find_resource() - Find a resource using match function/data. * @test: Test case to which the resource belongs. * @match: match function to be applied to resources/match data. @@ -345,6 +378,9 @@ struct kunit_resource *kunit_find_resource(struct kunit *test, kunit_resource_match_t match, void *match_data); +#define kunit_find_named_resource(test, name) \ + kunit_find_resource(test, kunit_resource_name_match, (void *)name) + /** * kunit_destroy_resource() - Find a kunit_resource and destroy it. * @test: Test case to which the resource belongs. @@ -358,6 +394,8 @@ int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, void *match_data); +#define kunit_destroy_named_resource(test, name) \ + kunit_destroy_resource(test, kunit_resource_name_match, name) /** * kunit_remove_resource: remove resource from resource list associated with diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index b8bf36d..079c558 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -317,6 +317,42 @@ static void kunit_resource_test_static(struct kunit *test) KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); } +static void kunit_resource_test_named(struct kunit *test) +{ + struct kunit_resource res1, res2, *found = NULL; + struct kunit_test_resource_context ctx; + + KUNIT_EXPECT_EQ(test, + kunit_add_named_resource(test, NULL, NULL, &res1, + "resource_1", &ctx), + 0); + KUNIT_EXPECT_PTR_EQ(test, res1.data, (void *)&ctx); + + KUNIT_EXPECT_EQ(test, + kunit_add_named_resource(test, NULL, NULL, &res1, + "resource_1", &ctx), + -EEXIST); + + KUNIT_EXPECT_EQ(test, + kunit_add_named_resource(test, NULL, NULL, &res2, + "resource_2", &ctx), + 0); + + found = kunit_find_named_resource(test, "resource_1"); + + KUNIT_EXPECT_PTR_EQ(test, found, &res1); + + if (found) + kunit_put_resource(&res1); + + KUNIT_EXPECT_EQ(test, kunit_destroy_named_resource(test, "resource_2"), + 0); + + kunit_cleanup(test); + + KUNIT_EXPECT_TRUE(test, list_empty(&test->resources)); +} + static int kunit_resource_test_init(struct kunit *test) { struct kunit_test_resource_context *ctx = @@ -346,6 +382,7 @@ static void kunit_resource_test_exit(struct kunit *test) KUNIT_CASE(kunit_resource_test_cleanup_resources), KUNIT_CASE(kunit_resource_test_proper_free_ordering), KUNIT_CASE(kunit_resource_test_static), + KUNIT_CASE(kunit_resource_test_named), {} }; diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 132e9bf..86a4d9c 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -380,6 +380,57 @@ int kunit_add_resource(struct kunit *test, kunit_resource_init_t init, } EXPORT_SYMBOL_GPL(kunit_add_resource); +/* Used to initialize named resource. */ +struct kunit_name_data { + const char *name; + kunit_resource_init_t init; + void *data; +}; + +static int kunit_init_named_resource(struct kunit_resource *res, void *data) +{ + struct kunit_name_data *name_data = data; + + res->name = name_data->name; + res->data = name_data->data; + res->init = name_data->init; + + if (res->init) + return res->init(res, name_data->data); + + res->data = name_data->data; + + return 0; +} + +int kunit_add_named_resource(struct kunit *test, + kunit_resource_init_t init, + kunit_resource_free_t free, + struct kunit_resource *res, + const char *name, + void *data) +{ + struct kunit_name_data name_data; + struct kunit_resource *existing; + + if (!name) + return -EINVAL; + + existing = kunit_find_named_resource(test, name); + if (existing) { + kunit_put_resource(existing); + return -EEXIST; + } + + name_data.name = name; + name_data.init = init; + name_data.data = data; + + return kunit_add_resource(test, kunit_init_named_resource, free, res, + &name_data); +} +EXPORT_SYMBOL_GPL(kunit_add_named_resource); + struct kunit_resource *kunit_alloc_and_get_resource(struct kunit *test, kunit_resource_init_t init, kunit_resource_free_t free,
The kunit resources API allows for custom initialization and cleanup code (init/fini); here we use the init code to set the new "struct kunit_resource" "name" field, and call an additional init function if needed. Having a simple way to name resources is useful in cases such as multithreaded tests where a set of resources are shared among threads; a pointer to the "struct kunit *" test state then is all that is needed to retrieve and use named resources. Support is provided to add, find and destroy named resources; the latter two are simply wrappers that use a "match-by-name" callback. If an attempt to add a resource with a name that already exists is made kunit_add_named_resource() will return NULL. Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- include/kunit/test.h | 40 ++++++++++++++++++++++++++++++++++++++- lib/kunit/kunit-test.c | 37 ++++++++++++++++++++++++++++++++++++ lib/kunit/test.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 127 insertions(+), 1 deletion(-)