| Message ID | 20240927-leds_device_for_each_child_node_scoped-v1-13-95c0614b38c8@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | leds: switch to device_for_each_child_node_scoped() | expand |
Fri, Sep 27, 2024 at 01:21:04AM +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', as an immediate return is possible. ... > if (ret || reg >= chipdef->n_leds) { > dev_err(dev, "Invalid 'reg' property for node %pfw\n", > child); > - ret = -EINVAL; > - goto err; > + return -EINVAL; > } I'm not sure how interpret this message, but the problem here is the shadowing of the original error code. Hence I would split this: if (ret) return ret; // possibly return dev_err_probe() with another message if (reg >= chipdef->n_leds) return dev_err_probe(dev, -EINVAL, "Invalid 'reg' property for node %pfw\n", child); ... > if (ret) { > dev_err(dev, "Failed to register LED for node %pfw\n", > child); > - goto err; > + return ret; return dev_err_probe(...); > }
diff --git a/drivers/leds/leds-pca963x.c b/drivers/leds/leds-pca963x.c index b53905da3592..050e93b04884 100644 --- a/drivers/leds/leds-pca963x.c +++ b/drivers/leds/leds-pca963x.c @@ -306,7 +306,6 @@ static int pca963x_register_leds(struct i2c_client *client, struct pca963x_chipdef *chipdef = chip->chipdef; struct pca963x_led *led = chip->leds; struct device *dev = &client->dev; - struct fwnode_handle *child; bool hw_blink; s32 mode2; u32 reg; @@ -338,7 +337,7 @@ static int pca963x_register_leds(struct i2c_client *client, if (ret < 0) return ret; - device_for_each_child_node(dev, child) { + device_for_each_child_node_scoped(dev, child) { struct led_init_data init_data = {}; char default_label[32]; @@ -346,8 +345,7 @@ static int pca963x_register_leds(struct i2c_client *client, if (ret || reg >= chipdef->n_leds) { dev_err(dev, "Invalid 'reg' property for node %pfw\n", child); - ret = -EINVAL; - goto err; + return -EINVAL; } led->led_num = reg; @@ -369,16 +367,13 @@ static int pca963x_register_leds(struct i2c_client *client, if (ret) { dev_err(dev, "Failed to register LED for node %pfw\n", child); - goto err; + return ret; } ++led; } return 0; -err: - fwnode_handle_put(child); - return ret; } static int pca963x_suspend(struct device *dev)
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', as an immediate return is possible. Signed-off-by: Javier Carrasco <javier.carrasco.cruz@gmail.com> --- drivers/leds/leds-pca963x.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)