| Message ID | 20240418131754.58217-3-wander@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Brendan Higgins |
| Headers | show |
| Series | kunit: fix minor error path mistakes | expand |
> 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@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+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources Regards, Markus
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. Signed-off-by: Wander Lairson Costa <wander@redhat.com> --- lib/kunit/device.c | 1 + 1 file changed, 1 insertion(+)