* [PATCH 0/2] kunit: fix minor error path mistakes
@ 2024-04-18 13:17 Wander Lairson Costa
2024-04-18 13:17 ` [PATCH 1/2] kunit: unregister the device on error Wander Lairson Costa
2024-04-18 13:17 ` [PATCH 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
0 siblings, 2 replies; 8+ messages in thread
From: Wander Lairson Costa @ 2024-04-18 13:17 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list
Cc: Wander Lairson Costa
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] kunit: unregister the device on error
2024-04-18 13:17 [PATCH 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
@ 2024-04-18 13:17 ` Wander Lairson Costa
2024-04-18 15:00 ` Markus Elfring
2024-04-18 13:17 ` [PATCH 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
1 sibling, 1 reply; 8+ messages in thread
From: Wander Lairson Costa @ 2024-04-18 13:17 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list
Cc: Wander Lairson Costa
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;
}
--
2.44.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] kunit: avoid memory leak on device register error
2024-04-18 13:17 [PATCH 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
2024-04-18 13:17 ` [PATCH 1/2] kunit: unregister the device on error Wander Lairson Costa
@ 2024-04-18 13:17 ` Wander Lairson Costa
2024-04-18 15:24 ` Markus Elfring
1 sibling, 1 reply; 8+ messages in thread
From: Wander Lairson Costa @ 2024-04-18 13:17 UTC (permalink / raw)
To: Brendan Higgins, David Gow, Rae Moar,
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list:KERNEL UNIT TESTING FRAMEWORK (KUnit),
open list
Cc: Wander Lairson Costa
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);
}
--
2.44.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] kunit: unregister the device on error
2024-04-18 13:17 ` [PATCH 1/2] kunit: unregister the device on error Wander Lairson Costa
@ 2024-04-18 15:00 ` Markus Elfring
2024-04-18 15:21 ` Wander Lairson Costa
0 siblings, 1 reply; 8+ messages in thread
From: Markus Elfring @ 2024-04-18 15:00 UTC (permalink / raw)
To: Wander Lairson Costa, kunit-dev, linux-kselftest,
kernel-janitors, Brendan Higgins, David Gow, Rae Moar
Cc: LKML
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] kunit: unregister the device on error
2024-04-18 15:00 ` Markus Elfring
@ 2024-04-18 15:21 ` Wander Lairson Costa
0 siblings, 0 replies; 8+ messages in thread
From: Wander Lairson Costa @ 2024-04-18 15:21 UTC (permalink / raw)
To: Markus Elfring
Cc: kunit-dev, linux-kselftest, kernel-janitors, Brendan Higgins,
David Gow, Rae Moar, LKML
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kunit: avoid memory leak on device register error
2024-04-18 13:17 ` [PATCH 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
@ 2024-04-18 15:24 ` Markus Elfring
2024-04-18 17:18 ` Wander Lairson Costa
0 siblings, 1 reply; 8+ messages in thread
From: Markus Elfring @ 2024-04-18 15:24 UTC (permalink / raw)
To: Wander Lairson Costa, kunit-dev, linux-kselftest,
kernel-janitors, Brendan Higgins, David Gow, Rae Moar
Cc: LKML
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] kunit: avoid memory leak on device register error
2024-04-18 15:24 ` Markus Elfring
@ 2024-04-18 17:18 ` Wander Lairson Costa
2024-04-18 18:08 ` [ " Markus Elfring
0 siblings, 1 reply; 8+ messages in thread
From: Wander Lairson Costa @ 2024-04-18 17:18 UTC (permalink / raw)
To: Markus Elfring
Cc: kunit-dev, linux-kselftest, kernel-janitors, Brendan Higgins,
David Gow, Rae Moar, LKML
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [ 2/2] kunit: avoid memory leak on device register error
2024-04-18 17:18 ` Wander Lairson Costa
@ 2024-04-18 18:08 ` Markus Elfring
0 siblings, 0 replies; 8+ messages in thread
From: Markus Elfring @ 2024-04-18 18:08 UTC (permalink / raw)
To: Wander Lairson Costa, kunit-dev, linux-kselftest,
kernel-janitors, Brendan Higgins, David Gow, Rae Moar
Cc: LKML
>> 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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-04-18 18:08 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-18 13:17 [PATCH 0/2] kunit: fix minor error path mistakes Wander Lairson Costa
2024-04-18 13:17 ` [PATCH 1/2] kunit: unregister the device on error Wander Lairson Costa
2024-04-18 15:00 ` Markus Elfring
2024-04-18 15:21 ` Wander Lairson Costa
2024-04-18 13:17 ` [PATCH 2/2] kunit: avoid memory leak on device register error Wander Lairson Costa
2024-04-18 15:24 ` Markus Elfring
2024-04-18 17:18 ` Wander Lairson Costa
2024-04-18 18:08 ` [ " Markus Elfring
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.