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!