Re: [PATCH] leds: leds-max77650: Add of_node_put() before return

[Nishka Dasgupta <nishkadg.linux@gmail.com>]

On 11/07/19 12:40 PM, Bartosz Golaszewski wrote:
> wt., 9 lip 2019 o 19:35 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 add an of_node_put before the
>> return in five places.
>> Issue found with Coccinelle.
>>
>> Signed-off-by: Nishka Dasgupta <nishkadg.linux@gmail.com>
>> ---
>>   drivers/leds/leds-max77650.c | 20 +++++++++++++++-----
>>   1 file changed, 15 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c
>> index 6b74ce9cac12..417820f77c32 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", &reg);
>> -               if (rv || reg >= MAX77650_LED_NUM_LEDS)
>> +               if (rv || reg >= MAX77650_LED_NUM_LEDS) {
>> +                       of_node_put(child);
>>                          return -EINVAL;
>> +               }
>>
>>                  led = &leds[reg];
>>                  led->map = map;
>> @@ -109,24 +111,32 @@ 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)
>> +                       if (!led->cdev.name) {
>> +                               of_node_put(child);
>>                                  return -ENOMEM;
>> +                       }
>>                  }
>>
>>                  of_property_read_string(child, "linux,default-trigger",
>>                                          &led->cdev.default_trigger);
>>
>>                  rv = devm_of_led_classdev_register(dev, child, &led->cdev);
>> -               if (rv)
>> +               if (rv) {
>> +                       of_node_put(child);
>>                          return rv;
>> +               }
>>
>>                  rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT);
>> -               if (rv)
>> +               if (rv) {
>> +                       of_node_put(child);
>>                          return rv;
>> +               }
>>
>>                  rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT);
>> -               if (rv)
>> +               if (rv) {
>> +                       of_node_put(child);
>>                          return rv;
>> +               }
>>          }
>>
>>          return regmap_write(map,
>> --
>> 2.19.1
>>
> 
> How about a label at the end of probe instead of adding this before
> every return?

I'll do that and send a version 2.

Thanking you,
Nishka

> Bart
>