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(-)
--
2.44.0
kunit_init_device() should unregister the device on bus register error.
Signed-off-by: Wander Lairson Costa <[email protected]>
---
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;
}
--
2.44.0
If the device register fails, free the allocated memory before
returning.
Signed-off-by: Wander Lairson Costa <[email protected]>
---
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);
}
--
2.44.0
> kunit_init_device() should unregister the device on bus register error.
* Would another imperative wording be desirable also for this change description?
* Will the tag “Fixes” become relevant here?
Regards,
Markus
On Thu, Apr 18, 2024 at 12:06 PM Markus Elfring <Markus.Elfring@webde> 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.
* 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?
Regards,
Markus
On Thu, Apr 18, 2024 at 12:24 PM Markus Elfring <Markus.Elfring@webde> 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+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources
Regards,
Markus