2022-05-17 01:18:40

by Daniel Latypov

[permalink] [raw]
Subject: [PATCH] kunit: use kmemdup in kunit_filter_tests(), take suite as const

kmemdup() is easier than kmalloc() + memcpy(), per lkp bot.

Also make the input `suite` as const since we're now always making
copies after commit a127b154a8f2 ("kunit: tool: allow filtering test
cases via glob").

Reported-by: kernel test robot <[email protected]>
Signed-off-by: Daniel Latypov <[email protected]>
---
lib/kunit/executor.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
index 2f73a6a35a7e..43e15112460a 100644
--- a/lib/kunit/executor.c
+++ b/lib/kunit/executor.c
@@ -55,7 +55,7 @@ static void kunit_parse_filter_glob(struct kunit_test_filter *parsed,

/* Create a copy of suite with only tests that match test_glob. */
static struct kunit_suite *
-kunit_filter_tests(struct kunit_suite *const suite, const char *test_glob)
+kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob)
{
int n = 0;
struct kunit_case *filtered, *test_case;
@@ -69,11 +69,9 @@ kunit_filter_tests(struct kunit_suite *const suite, const char *test_glob)
if (n == 0)
return NULL;

- /* Use memcpy to workaround copy->name being const. */
- copy = kmalloc(sizeof(*copy), GFP_KERNEL);
+ copy = kmemdup(suite, sizeof(*copy), GFP_KERNEL);
if (!copy)
return ERR_PTR(-ENOMEM);
- memcpy(copy, suite, sizeof(*copy));

filtered = kcalloc(n + 1, sizeof(*filtered), GFP_KERNEL);
if (!filtered)

base-commit: 9660209d9418f2295d31fea0d32e313e9b2c1200
--
2.36.0.550.gb090851708-goog



2022-05-18 08:00:55

by David Gow

[permalink] [raw]
Subject: Re: [PATCH] kunit: use kmemdup in kunit_filter_tests(), take suite as const

On Tue, May 17, 2022 at 12:54 AM Daniel Latypov <[email protected]> wrote:
>
> kmemdup() is easier than kmalloc() + memcpy(), per lkp bot.
>
> Also make the input `suite` as const since we're now always making
> copies after commit a127b154a8f2 ("kunit: tool: allow filtering test
> cases via glob").
>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Daniel Latypov <[email protected]>
> ---

Ah, this is definitely better. Thanks. I ran it under KASAN to double
check, and no issues there, either.

Reviewed-by: David Gow <[email protected]>

Cheers,
-- David

> lib/kunit/executor.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/lib/kunit/executor.c b/lib/kunit/executor.c
> index 2f73a6a35a7e..43e15112460a 100644
> --- a/lib/kunit/executor.c
> +++ b/lib/kunit/executor.c
> @@ -55,7 +55,7 @@ static void kunit_parse_filter_glob(struct kunit_test_filter *parsed,
>
> /* Create a copy of suite with only tests that match test_glob. */
> static struct kunit_suite *
> -kunit_filter_tests(struct kunit_suite *const suite, const char *test_glob)
> +kunit_filter_tests(const struct kunit_suite *const suite, const char *test_glob)
> {
> int n = 0;
> struct kunit_case *filtered, *test_case;
> @@ -69,11 +69,9 @@ kunit_filter_tests(struct kunit_suite *const suite, const char *test_glob)
> if (n == 0)
> return NULL;
>
> - /* Use memcpy to workaround copy->name being const. */
> - copy = kmalloc(sizeof(*copy), GFP_KERNEL);
> + copy = kmemdup(suite, sizeof(*copy), GFP_KERNEL);
> if (!copy)
> return ERR_PTR(-ENOMEM);
> - memcpy(copy, suite, sizeof(*copy));
>
> filtered = kcalloc(n + 1, sizeof(*filtered), GFP_KERNEL);
> if (!filtered)
>
> base-commit: 9660209d9418f2295d31fea0d32e313e9b2c1200
> --
> 2.36.0.550.gb090851708-goog
>


Attachments:
smime.p7s (3.91 kB)
S/MIME Cryptographic Signature

2022-07-06 19:23:02

by Brendan Higgins

[permalink] [raw]
Subject: Re: [PATCH] kunit: use kmemdup in kunit_filter_tests(), take suite as const

On Mon, May 16, 2022 at 12:54 PM Daniel Latypov <[email protected]> wrote:
>
> kmemdup() is easier than kmalloc() + memcpy(), per lkp bot.
>
> Also make the input `suite` as const since we're now always making
> copies after commit a127b154a8f2 ("kunit: tool: allow filtering test
> cases via glob").
>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Daniel Latypov <[email protected]>

Reviewed-by: Brendan Higgins <[email protected]>