* [cocci] [PATCH] leds: trigger: oneshot: One function call less in pattern_init() after error detection
@ 2023-12-26 21:08 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2023-12-26 21:08 UTC (permalink / raw)
To: linux-leds, kernel-janitors, Lee Jones, Pavel Machek; +Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 26 Dec 2023 22:02:08 +0100
The kfree() function was called in one case by
the pattern_init() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus use another label.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/leds/trigger/ledtrig-oneshot.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
index bee3bd452abf..31061ec0afe6 100644
--- a/drivers/leds/trigger/ledtrig-oneshot.c
+++ b/drivers/leds/trigger/ledtrig-oneshot.c
@@ -134,7 +134,7 @@ static void pattern_init(struct led_classdev *led_cdev)
pattern = led_get_default_pattern(led_cdev, &size);
if (!pattern)
- goto out_default;
+ goto out_settings;
if (size != 2) {
dev_warn(led_cdev->dev,
@@ -151,6 +151,7 @@ static void pattern_init(struct led_classdev *led_cdev)
out_default:
kfree(pattern);
+out_settings:
led_cdev->blink_delay_on = DEFAULT_DELAY;
led_cdev->blink_delay_off = DEFAULT_DELAY;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] leds: trigger: oneshot: One function call less in pattern_init() after error detection
@ 2023-12-26 21:08 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2023-12-26 21:08 UTC (permalink / raw)
To: linux-leds, kernel-janitors, Lee Jones, Pavel Machek; +Cc: LKML, cocci
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 26 Dec 2023 22:02:08 +0100
The kfree() function was called in one case by
the pattern_init() function during error handling
even if the passed variable contained a null pointer.
This issue was detected by using the Coccinelle software.
Thus use another label.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/leds/trigger/ledtrig-oneshot.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
index bee3bd452abf..31061ec0afe6 100644
--- a/drivers/leds/trigger/ledtrig-oneshot.c
+++ b/drivers/leds/trigger/ledtrig-oneshot.c
@@ -134,7 +134,7 @@ static void pattern_init(struct led_classdev *led_cdev)
pattern = led_get_default_pattern(led_cdev, &size);
if (!pattern)
- goto out_default;
+ goto out_settings;
if (size != 2) {
dev_warn(led_cdev->dev,
@@ -151,6 +151,7 @@ static void pattern_init(struct led_classdev *led_cdev)
out_default:
kfree(pattern);
+out_settings:
led_cdev->blink_delay_on = DEFAULT_DELAY;
led_cdev->blink_delay_off = DEFAULT_DELAY;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] leds: trigger: oneshot: One function call less in pattern_init() after error detection
2023-12-26 21:08 ` Markus Elfring
(?)
@ 2024-01-11 10:41 ` Lee Jones
2024-01-11 12:03 ` Markus Elfring
-1 siblings, 1 reply; 6+ messages in thread
From: Lee Jones @ 2024-01-11 10:41 UTC (permalink / raw)
To: Markus Elfring; +Cc: linux-leds, kernel-janitors, Pavel Machek, LKML, cocci
On Tue, 26 Dec 2023, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Tue, 26 Dec 2023 22:02:08 +0100
>
> The kfree() function was called in one case by
> the pattern_init() function during error handling
> even if the passed variable contained a null pointer.
It's totally valid to call kfree() on a NULL pointer:
* If @object is NULL, no operation is performed.
Why do we care all that much?
> This issue was detected by using the Coccinelle software.
>
> Thus use another label.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/leds/trigger/ledtrig-oneshot.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-oneshot.c b/drivers/leds/trigger/ledtrig-oneshot.c
> index bee3bd452abf..31061ec0afe6 100644
> --- a/drivers/leds/trigger/ledtrig-oneshot.c
> +++ b/drivers/leds/trigger/ledtrig-oneshot.c
> @@ -134,7 +134,7 @@ static void pattern_init(struct led_classdev *led_cdev)
>
> pattern = led_get_default_pattern(led_cdev, &size);
> if (!pattern)
> - goto out_default;
> + goto out_settings;
>
> if (size != 2) {
> dev_warn(led_cdev->dev,
> @@ -151,6 +151,7 @@ static void pattern_init(struct led_classdev *led_cdev)
>
> out_default:
> kfree(pattern);
> +out_settings:
> led_cdev->blink_delay_on = DEFAULT_DELAY;
> led_cdev->blink_delay_off = DEFAULT_DELAY;
> }
> --
> 2.43.0
>
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [cocci] leds: trigger: oneshot: One function call less in pattern_init() after error detection
2024-01-11 10:41 ` Lee Jones
@ 2024-01-11 12:03 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2024-01-11 12:03 UTC (permalink / raw)
To: Lee Jones, linux-leds, kernel-janitors; +Cc: Pavel Machek, LKML, cocci
>> The kfree() function was called in one case by
>> the pattern_init() function during error handling
>> even if the passed variable contained a null pointer.
>
> It's totally valid to call kfree() on a NULL pointer:
>
> * If @object is NULL, no operation is performed.
>
> Why do we care all that much?
Would you dare to categorise such special function calls as redundant?
Should they be skipped in more cases?
See also:
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] 6+ messages in thread
* Re: leds: trigger: oneshot: One function call less in pattern_init() after error detection
@ 2024-01-11 12:03 ` Markus Elfring
0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2024-01-11 12:03 UTC (permalink / raw)
To: Lee Jones, linux-leds, kernel-janitors; +Cc: Pavel Machek, LKML, cocci
>> The kfree() function was called in one case by
>> the pattern_init() function during error handling
>> even if the passed variable contained a null pointer.
>
> It's totally valid to call kfree() on a NULL pointer:
>
> * If @object is NULL, no operation is performed.
>
> Why do we care all that much?
Would you dare to categorise such special function calls as redundant?
Should they be skipped in more cases?
See also:
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] 6+ messages in thread
* Re: leds: trigger: oneshot: One function call less in pattern_init() after error detection
2024-01-11 12:03 ` Markus Elfring
(?)
@ 2024-01-11 13:05 ` Lee Jones
-1 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2024-01-11 13:05 UTC (permalink / raw)
To: Markus Elfring; +Cc: linux-leds, kernel-janitors, Pavel Machek, LKML, cocci
On Thu, 11 Jan 2024, Markus Elfring wrote:
> >> The kfree() function was called in one case by
> >> the pattern_init() function during error handling
> >> even if the passed variable contained a null pointer.
> >
> > It's totally valid to call kfree() on a NULL pointer:
> >
> > * If @object is NULL, no operation is performed.
> >
> > Why do we care all that much?
>
> Would you dare to categorise such special function calls as redundant?
>
> Should they be skipped in more cases?
>
> See also:
> 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
I have no idea what you're trying to say.
The premise of your patch is based on the fact that we shouldn't call
kfree() with a NULL pointer. When in actual fact kfree() is more than
capable of handling cases where the passed object is NULL, and goes as
far as to document as such. Meaning that unless I'm convinced
otherwise, patches like this remain in the category of churn.
--
Lee Jones [李琼斯]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-01-11 13:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-26 21:08 [cocci] [PATCH] leds: trigger: oneshot: One function call less in pattern_init() after error detection Markus Elfring
2023-12-26 21:08 ` Markus Elfring
2024-01-11 10:41 ` Lee Jones
2024-01-11 12:03 ` [cocci] " Markus Elfring
2024-01-11 12:03 ` Markus Elfring
2024-01-11 13:05 ` Lee Jones
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.