On Thu, Apr 22, 2021 at 2:32 AM Daniel Latypov dlatypov@google.com wrote:
Add in:
- kunit_kmalloc_array() and wire up kunit_kmalloc() to be a special
case of it.
- kunit_kcalloc() for symmetry with kunit_kzalloc()
This should using KUnit more natural by making it more similar to the existing *alloc() APIs.
And while we shouldn't necessarily be writing unit tests where overflow should be a concern, it can't hurt to be safe.
Signed-off-by: Daniel Latypov dlatypov@google.com
This seems like a good addition to me: a bug and a couple of useless asides below.
Apart from the "kzalloc"/"kcalloc" confusion in the comment below, this is Reviewed-by: David Gow davidgow@google.com
-- David
include/kunit/test.h | 36 ++++++++++++++++++++++++++++++++---- lib/kunit/test.c | 22 ++++++++++++---------- 2 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/include/kunit/test.h b/include/kunit/test.h index 49601c4b98b8..7fa0de4af977 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -577,16 +577,30 @@ static inline int kunit_destroy_named_resource(struct kunit *test, void kunit_remove_resource(struct kunit *test, struct kunit_resource *res);
/**
- kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
- kunit_kmalloc_array() - Like kmalloc_array() except the allocation is *test managed*.
- @test: The test context object.
- @n: number of elements.
- @size: The size in bytes of the desired memory.
- @gfp: flags passed to underlying kmalloc().
- Just like `kmalloc(...)`, except the allocation is managed by the test case
*/
- Just like `kmalloc_array(...)`, except the allocation is managed by the test case
- and is automatically cleaned up after the test case concludes. See &struct
- kunit_resource for more information.
-void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp); +void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t flags);
+/**
- kunit_kmalloc() - Like kmalloc() except the allocation is *test managed*.
- @test: The test context object.
- @size: The size in bytes of the desired memory.
- @gfp: flags passed to underlying kmalloc().
- See kmalloc() and kunit_kmalloc_array() for more information.
- */
+static inline void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) +{
return kunit_kmalloc_array(test, 1, size, gfp);
Do we want to implement kunit_kmalloc() in terms of kunit_kmalloc_array()? It's interestingly backwards given that kmalloc_array() is implemented in terms of kmalloc(). The other option would be to have each kunit_* function wrap the actual function that's called, but that would introduce a lot of code duplication for a very small performance benefit.
I'm happy with it the way it is now that I've looked through the implementations, but I was a little uneasy at first that some of these functions might not actually call the function they're theoretically wrapping.
+}
/**
- kunit_kfree() - Like kfree except for allocations managed by KUnit.
@@ -601,13 +615,27 @@ void kunit_kfree(struct kunit *test, const void *ptr);
- @size: The size in bytes of the desired memory.
- @gfp: flags passed to underlying kmalloc().
- See kzalloc() and kunit_kmalloc() for more information.
*/
- See kzalloc() and kunit_kmalloc_array() for more information.
static inline void *kunit_kzalloc(struct kunit *test, size_t size, gfp_t gfp) { return kunit_kmalloc(test, size, gfp | __GFP_ZERO); }
+/**
- kunit_kzalloc() - Just like kunit_kmalloc_array(), but zeroes the allocation.
The function is called kunit_kcalloc(), but the documentation comment calls it kunit_kzalloc(). Copy + paste error from above?
- @test: The test context object.
- @n: number of elements.
- @size: The size in bytes of the desired memory.
- @gfp: flags passed to underlying kmalloc().
- See kcalloc() and kunit_kmalloc_array() for more information.
- */
+static inline void *kunit_kcalloc(struct kunit *test, size_t n, size_t size, gfp_t flags) +{
return kunit_kmalloc_array(test, n, size, flags | __GFP_ZERO);+}
void kunit_cleanup(struct kunit *test);
void kunit_log_append(char *log, const char *fmt, ...); diff --git a/lib/kunit/test.c b/lib/kunit/test.c index ec9494e914ef..052fccf69eef 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -540,41 +540,43 @@ int kunit_destroy_resource(struct kunit *test, kunit_resource_match_t match, } EXPORT_SYMBOL_GPL(kunit_destroy_resource);
-struct kunit_kmalloc_params { +struct kunit_kmalloc_array_params {
size_t n;
It's worth noting that we never actually use this after the resource is created. kmalloc_array() discards 'n' after the overflow check and multiply anyway.
Of course, we don't need 'size' either, and we were already tracking that. I guess it's just an overhead of the resource system, so nothing worth actually changing here.
size_t size; gfp_t gfp;};
-static int kunit_kmalloc_init(struct kunit_resource *res, void *context) +static int kunit_kmalloc_array_init(struct kunit_resource *res, void *context) {
struct kunit_kmalloc_params *params = context;
struct kunit_kmalloc_array_params *params = context;
res->data = kmalloc(params->size, params->gfp);
res->data = kmalloc_array(params->n, params->size, params->gfp); if (!res->data) return -ENOMEM; return 0;}
-static void kunit_kmalloc_free(struct kunit_resource *res) +static void kunit_kmalloc_array_free(struct kunit_resource *res) { kfree(res->data); }
-void *kunit_kmalloc(struct kunit *test, size_t size, gfp_t gfp) +void *kunit_kmalloc_array(struct kunit *test, size_t n, size_t size, gfp_t gfp) {
struct kunit_kmalloc_params params = {
struct kunit_kmalloc_array_params params = { .size = size,.n = n, .gfp = gfp }; return kunit_alloc_resource(test,
kunit_kmalloc_init,kunit_kmalloc_free,
kunit_kmalloc_array_init,kunit_kmalloc_array_free, gfp, ¶ms);} -EXPORT_SYMBOL_GPL(kunit_kmalloc); +EXPORT_SYMBOL_GPL(kunit_kmalloc_array);
void kunit_kfree(struct kunit *test, const void *ptr) {
base-commit: 16fc44d6387e260f4932e9248b985837324705d8
2.31.1.498.g6c1eba8ee3d-goog