* [PATCH v2] leds: max77650: Add of_node_put() before return
@ 2019-07-11 7:44 Nishka Dasgupta
2019-07-11 7:54 ` [PATCH v3] " Nishka Dasgupta
0 siblings, 1 reply; 5+ messages in thread
From: Nishka Dasgupta @ 2019-07-11 7:44 UTC (permalink / raw)
To: bgolaszewski, jacek.anaszewski, pavel, dmurphy, linux-leds
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak.
Hence create a new label, err_node_put, which puts the previous node and
returns variable rv. Modify the mid-loop return statements to instead
store the return value in rv and jump to err_node_put.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
drivers/leds/leds-max77650.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
index 6b74ce9cac12..1eb9998899e4 100644
--- a/drivers/leds/leds-max77650.c
+++ b/drivers/leds/leds-max77650.c
@@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
for_each_child_of_node(of_node, child) {
rv = of_property_read_u32(child, "reg", ®);
- if (rv || reg >= MAX77650_LED_NUM_LEDS)
- return -EINVAL;
+ if (rv || reg >= MAX77650_LED_NUM_LEDS) {
+ rv = -EINVAL;
+ goto err_node_put;
+ }
led = &leds[reg];
led->map = map;
@@ -109,8 +111,10 @@ static int max77650_led_probe(struct platform_device *pdev)
} else {
led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
"max77650:%s", label);
- if (!led->cdev.name)
- return -ENOMEM;
+ if (!led->cdev.name) {
+ rv = -ENOMEM;
+ goto err_node_put;
+ }
}
of_property_read_string(child, "linux,default-trigger",
@@ -118,20 +122,23 @@ static int max77650_led_probe(struct platform_device *pdev)
rv = devm_of_led_classdev_register(dev, child, &led->cdev);
if (rv)
- return rv;
+ goto err_node_put;
rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
if (rv)
- return rv;
+ goto err_node_put;
rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
if (rv)
- return rv;
+ goto err_node_put;
}
return regmap_write(map,
MAX77650_REG_CNFG_LED_TOP,
MAX77650_LED_TOP_DEFAULT);
+err_node_put:
+ of_node_put(child);
+ return rv;
}
static struct platform_driver max77650_led_driver = {
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3] leds: max77650: Add of_node_put() before return
2019-07-11 7:44 [PATCH v2] leds: max77650: Add of_node_put() before return Nishka Dasgupta
@ 2019-07-11 7:54 ` Nishka Dasgupta
2019-07-12 14:12 ` Dan Murphy
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Nishka Dasgupta @ 2019-07-11 7:54 UTC (permalink / raw)
To: bgolaszewski, jacek.anaszewski, pavel, dmurphy, linux-leds
Cc: Nishka Dasgupta
Each iteration of for_each_child_of_node puts the previous node, but in
the case of a return from the middle of the loop, there is no put, thus
causing a memory leak.
Hence create a new label, err_node_put, which puts the previous node and
returns variable rv. Modify the mid-loop return statements to instead
store the return value in rv and jump to err_node_put.
Issue found with Coccinelle.
Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
---
Changes in v3:
- Add change log.
Changes in v2:
- Change subject line to match previous patches on the same file.
- Merge the of_node_put calls into a single call in a label at the end
of the function instead of calling it separately for each return
statement.
drivers/leds/leds-max77650.c | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
index 6b74ce9cac12..1eb9998899e4 100644
--- a/drivers/leds/leds-max77650.c
+++ b/drivers/leds/leds-max77650.c
@@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
for_each_child_of_node(of_node, child) {
rv = of_property_read_u32(child, "reg", ®);
- if (rv || reg >= MAX77650_LED_NUM_LEDS)
- return -EINVAL;
+ if (rv || reg >= MAX77650_LED_NUM_LEDS) {
+ rv = -EINVAL;
+ goto err_node_put;
+ }
led = &leds[reg];
led->map = map;
@@ -109,8 +111,10 @@ static int max77650_led_probe(struct platform_device *pdev)
} else {
led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
"max77650:%s", label);
- if (!led->cdev.name)
- return -ENOMEM;
+ if (!led->cdev.name) {
+ rv = -ENOMEM;
+ goto err_node_put;
+ }
}
of_property_read_string(child, "linux,default-trigger",
@@ -118,20 +122,23 @@ static int max77650_led_probe(struct platform_device *pdev)
rv = devm_of_led_classdev_register(dev, child, &led->cdev);
if (rv)
- return rv;
+ goto err_node_put;
rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
if (rv)
- return rv;
+ goto err_node_put;
rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
if (rv)
- return rv;
+ goto err_node_put;
}
return regmap_write(map,
MAX77650_REG_CNFG_LED_TOP,
MAX77650_LED_TOP_DEFAULT);
+err_node_put:
+ of_node_put(child);
+ return rv;
}
static struct platform_driver max77650_led_driver = {
--
2.19.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] leds: max77650: Add of_node_put() before return
2019-07-11 7:54 ` [PATCH v3] " Nishka Dasgupta
@ 2019-07-12 14:12 ` Dan Murphy
2019-07-12 14:15 ` Bartosz Golaszewski
2019-07-15 21:02 ` Jacek Anaszewski
2 siblings, 0 replies; 5+ messages in thread
From: Dan Murphy @ 2019-07-12 14:12 UTC (permalink / raw)
To: Nishka Dasgupta, bgolaszewski, jacek.anaszewski, pavel, linux-leds
On 7/11/19 2:54 AM, Nishka Dasgupta wrote:
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak.
> Hence create a new label, err_node_put, which puts the previous node and
> returns variable rv. Modify the mid-loop return statements to instead
> store the return value in rv and jump to err_node_put.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
Reviewed-by: Dan Murphy <dmurphy@ti.com>
> ---
> Changes in v3:
> - Add change log.
> Changes in v2:
> - Change subject line to match previous patches on the same file.
> - Merge the of_node_put calls into a single call in a label at the end
> of the function instead of calling it separately for each return
> statement.
>
> drivers/leds/leds-max77650.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> index 6b74ce9cac12..1eb9998899e4 100644
> --- a/drivers/leds/leds-max77650.c
> +++ b/drivers/leds/leds-max77650.c
> @@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> for_each_child_of_node(of_node, child) {
> rv = of_property_read_u32(child, "reg", ®);
> - if (rv || reg >= MAX77650_LED_NUM_LEDS)
> - return -EINVAL;
> + if (rv || reg >= MAX77650_LED_NUM_LEDS) {
> + rv = -EINVAL;
> + goto err_node_put;
> + }
>
> led = &leds[reg];
> led->map = map;
> @@ -109,8 +111,10 @@ static int max77650_led_probe(struct platform_device *pdev)
> } else {
> led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> "max77650:%s", label);
> - if (!led->cdev.name)
> - return -ENOMEM;
> + if (!led->cdev.name) {
> + rv = -ENOMEM;
> + goto err_node_put;
> + }
> }
>
> of_property_read_string(child, "linux,default-trigger",
> @@ -118,20 +122,23 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> rv = devm_of_led_classdev_register(dev, child, &led->cdev);
> if (rv)
> - return rv;
> + goto err_node_put;
>
> rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
> if (rv)
> - return rv;
> + goto err_node_put;
>
> rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
> if (rv)
> - return rv;
> + goto err_node_put;
> }
>
> return regmap_write(map,
> MAX77650_REG_CNFG_LED_TOP,
> MAX77650_LED_TOP_DEFAULT);
> +err_node_put:
> + of_node_put(child);
> + return rv;
> }
>
> static struct platform_driver max77650_led_driver = {
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] leds: max77650: Add of_node_put() before return
2019-07-11 7:54 ` [PATCH v3] " Nishka Dasgupta
2019-07-12 14:12 ` Dan Murphy
@ 2019-07-12 14:15 ` Bartosz Golaszewski
2019-07-15 21:02 ` Jacek Anaszewski
2 siblings, 0 replies; 5+ messages in thread
From: Bartosz Golaszewski @ 2019-07-12 14:15 UTC (permalink / raw)
To: Nishka Dasgupta
Cc: Jacek Anaszewski, Pavel Machek, dmurphy, Linux LED Subsystem
czw., 11 lip 2019 o 09:54 Nishka Dasgupta <nishkadg.linux@gmail.com> napisał(a):
>
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak.
> Hence create a new label, err_node_put, which puts the previous node and
> returns variable rv. Modify the mid-loop return statements to instead
> store the return value in rv and jump to err_node_put.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> Changes in v3:
> - Add change log.
> Changes in v2:
> - Change subject line to match previous patches on the same file.
> - Merge the of_node_put calls into a single call in a label at the end
> of the function instead of calling it separately for each return
> statement.
>
> drivers/leds/leds-max77650.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> index 6b74ce9cac12..1eb9998899e4 100644
> --- a/drivers/leds/leds-max77650.c
> +++ b/drivers/leds/leds-max77650.c
> @@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> for_each_child_of_node(of_node, child) {
> rv = of_property_read_u32(child, "reg", ®);
> - if (rv || reg >= MAX77650_LED_NUM_LEDS)
> - return -EINVAL;
> + if (rv || reg >= MAX77650_LED_NUM_LEDS) {
> + rv = -EINVAL;
> + goto err_node_put;
> + }
>
> led = &leds[reg];
> led->map = map;
> @@ -109,8 +111,10 @@ static int max77650_led_probe(struct platform_device *pdev)
> } else {
> led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> "max77650:%s", label);
> - if (!led->cdev.name)
> - return -ENOMEM;
> + if (!led->cdev.name) {
> + rv = -ENOMEM;
> + goto err_node_put;
> + }
> }
>
> of_property_read_string(child, "linux,default-trigger",
> @@ -118,20 +122,23 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> rv = devm_of_led_classdev_register(dev, child, &led->cdev);
> if (rv)
> - return rv;
> + goto err_node_put;
>
> rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
> if (rv)
> - return rv;
> + goto err_node_put;
>
> rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
> if (rv)
> - return rv;
> + goto err_node_put;
> }
>
> return regmap_write(map,
> MAX77650_REG_CNFG_LED_TOP,
> MAX77650_LED_TOP_DEFAULT);
> +err_node_put:
> + of_node_put(child);
> + return rv;
> }
>
> static struct platform_driver max77650_led_driver = {
> --
> 2.19.1
>
Acked-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] leds: max77650: Add of_node_put() before return
2019-07-11 7:54 ` [PATCH v3] " Nishka Dasgupta
2019-07-12 14:12 ` Dan Murphy
2019-07-12 14:15 ` Bartosz Golaszewski
@ 2019-07-15 21:02 ` Jacek Anaszewski
2 siblings, 0 replies; 5+ messages in thread
From: Jacek Anaszewski @ 2019-07-15 21:02 UTC (permalink / raw)
To: Nishka Dasgupta, bgolaszewski, pavel, dmurphy, linux-leds
Hi Nishka,
Thank you for the patch.
On 7/11/19 9:54 AM, Nishka Dasgupta wrote:
> Each iteration of for_each_child_of_node puts the previous node, but in
> the case of a return from the middle of the loop, there is no put, thus
> causing a memory leak.
> Hence create a new label, err_node_put, which puts the previous node and
> returns variable rv. Modify the mid-loop return statements to instead
> store the return value in rv and jump to err_node_put.
> Issue found with Coccinelle.
>
> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
> ---
> Changes in v3:
> - Add change log.
> Changes in v2:
> - Change subject line to match previous patches on the same file.
> - Merge the of_node_put calls into a single call in a label at the end
> of the function instead of calling it separately for each return
> statement.
>
> drivers/leds/leds-max77650.c | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
> index 6b74ce9cac12..1eb9998899e4 100644
> --- a/drivers/leds/leds-max77650.c
> +++ b/drivers/leds/leds-max77650.c
> @@ -93,8 +93,10 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> for_each_child_of_node(of_node, child) {
> rv = of_property_read_u32(child, "reg", ®);
> - if (rv || reg >= MAX77650_LED_NUM_LEDS)
> - return -EINVAL;
> + if (rv || reg >= MAX77650_LED_NUM_LEDS) {
> + rv = -EINVAL;
> + goto err_node_put;
> + }
>
> led = &leds[reg];
> led->map = map;
> @@ -109,8 +111,10 @@ static int max77650_led_probe(struct platform_device *pdev)
> } else {
> led->cdev.name = devm_kasprintf(dev, GFP_KERNEL,
> "max77650:%s", label);
> - if (!led->cdev.name)
> - return -ENOMEM;
> + if (!led->cdev.name) {
> + rv = -ENOMEM;
> + goto err_node_put;
> + }
> }
>
> of_property_read_string(child, "linux,default-trigger",
> @@ -118,20 +122,23 @@ static int max77650_led_probe(struct platform_device *pdev)
>
> rv = devm_of_led_classdev_register(dev, child, &led->cdev);
> if (rv)
> - return rv;
> + goto err_node_put;
>
> rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
> if (rv)
> - return rv;
> + goto err_node_put;
>
> rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
> if (rv)
> - return rv;
> + goto err_node_put;
> }
>
> return regmap_write(map,
> MAX77650_REG_CNFG_LED_TOP,
> MAX77650_LED_TOP_DEFAULT);
> +err_node_put:
> + of_node_put(child);
> + return rv;
> }
>
> static struct platform_driver max77650_led_driver = {
>
Applied to for-5.4 branch of linux-leds.git.
--
Best regards,
Jacek Anaszewski
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-07-15 21:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-11 7:44 [PATCH v2] leds: max77650: Add of_node_put() before return Nishka Dasgupta
2019-07-11 7:54 ` [PATCH v3] " Nishka Dasgupta
2019-07-12 14:12 ` Dan Murphy
2019-07-12 14:15 ` Bartosz Golaszewski
2019-07-15 21:02 ` Jacek Anaszewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).