| Message ID | 20240927-leds_device_for_each_child_node_scoped-v1-11-95c0614b38c8@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | leds: switch to device_for_each_child_node_scoped() | expand |
On Fri, Sep 27, 2024 at 1:21 AM Javier Carrasco <javier.carrasco.cruz@gmail.com> wrote: > > Switch to device_for_each_child_node_scoped() to simplify the code by > removing the need for calls to fwnode_handle_put() in the error paths. > > This also prevents possible memory leaks if new error paths are added > without the required call to fwnode_handle_put(). > > After switching to the scoped variant, there is no longer need for a > jump to 'err_node_out', as an immediate return is possible. > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> > --- Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Fri, Sep 27, 2024 at 01:21:02AM +0200, Javier Carrasco kirjoitti: > Switch to device_for_each_child_node_scoped() to simplify the code by > removing the need for calls to fwnode_handle_put() in the error paths. > > This also prevents possible memory leaks if new error paths are added > without the required call to fwnode_handle_put(). > > After switching to the scoped variant, there is no longer need for a > jump to 'err_node_out', as an immediate return is possible. ... > rv = fwnode_property_read_u32(child, "reg", ®); > - if (rv || reg >= MAX77650_LED_NUM_LEDS) { > - rv = -EINVAL; > - goto err_node_put; > - } > + if (rv || reg >= MAX77650_LED_NUM_LEDS) > + return -EINVAL; Again (yes, I know that is original issue, but with your series applied it may be nicely resolved), shadowing error code. Should be if (rv) return rv; if (reg >= MAX77650_LED_NUM_LEDS) return -EINVAL;
diff --git a/drivers/leds/leds-max77650.c b/drivers/leds/leds-max77650.c index 1eeac56b0014..f8c47078a3bb 100644 --- a/drivers/leds/leds-max77650.c +++ b/drivers/leds/leds-max77650.c @@ -62,7 +62,6 @@ static int max77650_led_brightness_set(struct led_classdev *cdev, static int max77650_led_probe(struct platform_device *pdev) { - struct fwnode_handle *child; struct max77650_led *leds, *led; struct device *dev; struct regmap *map; @@ -84,14 +83,12 @@ static int max77650_led_probe(struct platform_device *pdev) if (!num_leds || num_leds > MAX77650_LED_NUM_LEDS) return -ENODEV; - device_for_each_child_node(dev, child) { + device_for_each_child_node_scoped(dev, child) { struct led_init_data init_data = {}; rv = fwnode_property_read_u32(child, "reg", ®); - if (rv || reg >= MAX77650_LED_NUM_LEDS) { - rv = -EINVAL; - goto err_node_put; - } + if (rv || reg >= MAX77650_LED_NUM_LEDS) + return -EINVAL; led = &leds[reg]; led->map = map; @@ -108,23 +105,20 @@ static int max77650_led_probe(struct platform_device *pdev) rv = devm_led_classdev_register_ext(dev, &led->cdev, &init_data); if (rv) - goto err_node_put; + return rv; rv = regmap_write(map, led->regA, MAX77650_LED_A_DEFAULT); if (rv) - goto err_node_put; + return rv; rv = regmap_write(map, led->regB, MAX77650_LED_B_DEFAULT); if (rv) - goto err_node_put; + return rv; } return regmap_write(map, MAX77650_REG_CNFG_LED_TOP, MAX77650_LED_TOP_DEFAULT); -err_node_put: - fwnode_handle_put(child); - return rv; } static const struct of_device_id max77650_led_of_match[] = {
Switch to device_for_each_child_node_scoped() to simplify the code by removing the need for calls to fwnode_handle_put() in the error paths. This also prevents possible memory leaks if new error paths are added without the required call to fwnode_handle_put(). After switching to the scoped variant, there is no longer need for a jump to 'err_node_out', as an immediate return is possible. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/leds/leds-max77650.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)