Hi,
These two patches fix some minor error path mistakes in the device module.
Wander Lairson Costa (2): kunit: unregister the device on error kunit: avoid memory leak on device register error
lib/kunit/device.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
kunit_init_device() should unregister the device on bus register error.
Signed-off-by: Wander Lairson Costa wander@redhat.com --- lib/kunit/device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/kunit/device.c b/lib/kunit/device.c index abc603730b8e..25c81ed465fb 100644 --- a/lib/kunit/device.c +++ b/lib/kunit/device.c @@ -51,7 +51,7 @@ int kunit_bus_init(void)
error = bus_register(&kunit_bus_type); if (error) - bus_unregister(&kunit_bus_type); + root_device_unregister(kunit_bus_device); return error; }
On Thu, Apr 18, 2024 at 12:06 PM Markus Elfring Markus.Elfring@web.de wrote:
kunit_init_device() should unregister the device on bus register error.
- Would another imperative wording be desirable also for this change description?
It makes sense, I will change the comment description.
- Will the tag “Fixes” become relevant here?
I often forget this tag. I will add it.
Regards, Markus
If the device register fails, free the allocated memory before returning.
Signed-off-by: Wander Lairson Costa wander@redhat.com --- lib/kunit/device.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/lib/kunit/device.c b/lib/kunit/device.c index 25c81ed465fb..d8c09dcb3e79 100644 --- a/lib/kunit/device.c +++ b/lib/kunit/device.c @@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test, err = device_register(&kunit_dev->dev); if (err) { put_device(&kunit_dev->dev); + kfree(kunit_dev); return ERR_PTR(err); }
If the device register fails, free the allocated memory before returning.
* I suggest to use the word “registration” (instead of “register”) in the commit message.
* Would you like to add the tag “Fixes” accordingly?
+++ b/lib/kunit/device.c @@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test, err = device_register(&kunit_dev->dev); if (err) { put_device(&kunit_dev->dev);
kfree(kunit_dev);
Common error handling code can be used instead if an additional label would be applied for a corresponding jump target.
How do you think about to increase the application of scope-based resource management here?
Regards, Markus
On Thu, Apr 18, 2024 at 12:24 PM Markus Elfring Markus.Elfring@web.de wrote:
If the device register fails, free the allocated memory before returning.
I suggest to use the word “registration” (instead of “register”) in the commit message.
Would you like to add the tag “Fixes” accordingly?
+++ b/lib/kunit/device.c @@ -131,6 +131,7 @@ static struct kunit_device *kunit_device_register_internal(struct kunit *test, err = device_register(&kunit_dev->dev); if (err) { put_device(&kunit_dev->dev);
kfree(kunit_dev); return ERR_PTR(err); }Common error handling code can be used instead if an additional label would be applied for a corresponding jump target.
How do you think about to increase the application of scope-based resource management here?
I thought about that. But I think the code is simple enough (for now) to not require an exit label.
Regards, Markus
Common error handling code can be used instead if an additional label would be applied for a corresponding jump target.
How do you think about to increase the application of scope-based resource management here?
I thought about that. But I think the code is simple enough (for now) to not require an exit label.
Please follow a known advice (besides other recommended improvements). https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto...
Regards, Markus
linux-kselftest-mirror@lists.linaro.org
-
Markus Elfring -
Wander Lairson Costa