The kunit_device_register() function doesn't return NULL, it returns error pointers. Change the KUNIT_ASSERT_NOT_NULL() to check for ERR_OR_NULL().
Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org --- It's a pity that there isn't a KUNIT_ASSERT_NOT_ERR_PTR() macro...
lib/kunit/kunit-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index c4259d910356..f7980ef236a3 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -720,7 +720,7 @@ static void kunit_device_cleanup_test(struct kunit *test) long action_was_run = 0;
test_device = kunit_device_register(test, "my_device"); - KUNIT_ASSERT_NOT_NULL(test, test_device); + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_device);
/* Add an action to verify cleanup. */ devm_add_action(test_device, test_dev_action, &action_was_run);
On Wed, Jan 10, 2024 at 1:55 PM Dan Carpenter dan.carpenter@linaro.org wrote:
The kunit_device_register() function doesn't return NULL, it returns error pointers. Change the KUNIT_ASSERT_NOT_NULL() to check for ERR_OR_NULL().
Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org
This change looks good to me! Thanks! -Rae
Reviewed-by: Rae Moar rmoar@google.com
It's a pity that there isn't a KUNIT_ASSERT_NOT_ERR_PTR() macro...
lib/kunit/kunit-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index c4259d910356..f7980ef236a3 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -720,7 +720,7 @@ static void kunit_device_cleanup_test(struct kunit *test) long action_was_run = 0;
test_device = kunit_device_register(test, "my_device");
KUNIT_ASSERT_NOT_NULL(test, test_device);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_device); /* Add an action to verify cleanup. */ devm_add_action(test_device, test_dev_action, &action_was_run);-- 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@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/39b4278f-35d2-4071-a3aa-ec497052....
On Thu, 11 Jan 2024 at 02:55, Dan Carpenter dan.carpenter@linaro.org wrote:
The kunit_device_register() function doesn't return NULL, it returns error pointers. Change the KUNIT_ASSERT_NOT_NULL() to check for ERR_OR_NULL().
Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org
Nice catch, thanks!
Reviewed-by: David Gow davidgow@google.com
It's a pity that there isn't a KUNIT_ASSERT_NOT_ERR_PTR() macro...
I think we'll add one, but I'm not yet totally convinced that it would be better than using ASSERT_NOT_ERR_OR_NULL() in cases like this, where we're: 1. In a test; and, 2. using the pointer afterwards, expecting it to be valid (dereferencing it and/or passing it to functions which will)
This is largely because it'd be nicer, if the pointer is NULL (due to a bug), to get a more explicit assertion failure, rather than a crash. It does make the test code less indicative of how the APIs are meant to be used elsewhere, though, and annoys the static analysis, though.
Thoughts?
-- David
lib/kunit/kunit-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/kunit-test.c b/lib/kunit/kunit-test.c index c4259d910356..f7980ef236a3 100644 --- a/lib/kunit/kunit-test.c +++ b/lib/kunit/kunit-test.c @@ -720,7 +720,7 @@ static void kunit_device_cleanup_test(struct kunit *test) long action_was_run = 0;
test_device = kunit_device_register(test, "my_device");
KUNIT_ASSERT_NOT_NULL(test, test_device);
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, test_device); /* Add an action to verify cleanup. */ devm_add_action(test_device, test_dev_action, &action_was_run);-- 2.43.0
On Fri, Jan 12, 2024 at 07:39:14AM +0800, David Gow wrote:
On Thu, 11 Jan 2024 at 02:55, Dan Carpenter dan.carpenter@linaro.org wrote:
The kunit_device_register() function doesn't return NULL, it returns error pointers. Change the KUNIT_ASSERT_NOT_NULL() to check for ERR_OR_NULL().
Fixes: d03c720e03bd ("kunit: Add APIs for managing devices") Signed-off-by: Dan Carpenter dan.carpenter@linaro.org
Nice catch, thanks!
Reviewed-by: David Gow davidgow@google.com
It's a pity that there isn't a KUNIT_ASSERT_NOT_ERR_PTR() macro...
I think we'll add one, but I'm not yet totally convinced that it would be better than using ASSERT_NOT_ERR_OR_NULL() in cases like this, where we're:
- In a test; and,
- using the pointer afterwards, expecting it to be valid
(dereferencing it and/or passing it to functions which will)
This is largely because it'd be nicer, if the pointer is NULL (due to a bug), to get a more explicit assertion failure, rather than a crash. It does make the test code less indicative of how the APIs are meant to be used elsewhere, though, and annoys the static analysis, though.
Thoughts?
It doesn't annoy any static checkers because nothing looks for it.
Expecting that this test code might be buggier than normal code probably isn't unreasonable so I guess that makes sense.
regards, dan carpenter
linux-kselftest-mirror@lists.linaro.org
-
Dan Carpenter -
David Gow -
Rae Moar