| Message ID | 20210810115303.9089-1-novikov@ispras.ru |
|---|---|
| State | Accepted |
| Commit | e16f5e39acd6d10cc63ae39bc0a77188ed828f22 |
| Headers | show |
| Series | media: atomisp: Fix error handling in probe | expand |
On Tue, Aug 10, 2021 at 02:53:03PM +0300, Evgeny Novikov wrote: > There were several issues with handling errors in lm3554_probe(): > - Probe did not set the error code when v4l2_ctrl_handler_init() failed. > - It intermixed gotos for handling errors of v4l2_ctrl_handler_init() > and media_entity_pads_init(). > - Probe did not free resources in case of failures of > atomisp_register_i2c_module(). > > The patch fixes all these issues. > > Found by Linux Driver Verification project (linuxtesting.org). This patch is good, but two things: 1) Please can you rename all the labels to say what the goto does? err_uninit: lm3554_gpio_uninit(client); cleanup_media: media_entity_cleanup(&flash->sd.entity); free_handler: v4l2_ctrl_handler_free(&flash->ctrl_handler); unregister_subdev: v4l2_device_unregister_subdev(&flash->sd); 2) There is a missing error code still: drivers/staging/media/atomisp/i2c/atomisp-lm3554.c 858 if (ret) { 859 dev_err(&client->dev, "error initialize a ctrl_handler.\n"); 860 goto fail3; 861 } 862 863 for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++) 864 v4l2_ctrl_new_custom(&flash->ctrl_handler, &lm3554_controls[i], 865 NULL); 866 867 if (flash->ctrl_handler.error) { 868 dev_err(&client->dev, "ctrl_handler error.\n"); 869 goto fail3; ^^^^^^^^^^ err = flash->ctrl_handler.error; 870 } 871 872 flash->sd.ctrl_handler = &flash->ctrl_handler; 873 err = media_entity_pads_init(&flash->sd.entity, 0, NULL); 874 if (err) { 875 dev_err(&client->dev, "error initialize a media entity.\n"); 876 goto fail2; 877 } regards, dan carpenter
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c index 362ed44b4eff..1e9432d6ec0c 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c @@ -835,7 +835,6 @@ static int lm3554_probe(struct i2c_client *client) int err = 0; struct lm3554 *flash; unsigned int i; - int ret; flash = kzalloc(sizeof(*flash), GFP_KERNEL); if (!flash) @@ -852,12 +851,12 @@ static int lm3554_probe(struct i2c_client *client) flash->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; flash->mode = ATOMISP_FLASH_MODE_OFF; flash->timeout = LM3554_MAX_TIMEOUT / LM3554_TIMEOUT_STEPSIZE - 1; - ret = + err = v4l2_ctrl_handler_init(&flash->ctrl_handler, ARRAY_SIZE(lm3554_controls)); - if (ret) { + if (err) { dev_err(&client->dev, "error initialize a ctrl_handler.\n"); - goto fail3; + goto fail2; } for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++) @@ -873,7 +872,7 @@ static int lm3554_probe(struct i2c_client *client) err = media_entity_pads_init(&flash->sd.entity, 0, NULL); if (err) { dev_err(&client->dev, "error initialize a media entity.\n"); - goto fail2; + goto fail3; } flash->sd.entity.function = MEDIA_ENT_F_FLASH; @@ -884,12 +883,23 @@ static int lm3554_probe(struct i2c_client *client) err = lm3554_gpio_init(client); if (err) { - dev_err(&client->dev, "gpio request/direction_output fail"); - goto fail3; + dev_err(&client->dev, "gpio request/direction_output fail.\n"); + goto fail4; } - return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); -fail3: + + err = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); + if (err) { + dev_err(&client->dev, "fail to register atomisp i2c module.\n"); + goto fail5; + } + + return 0; + +fail5: + lm3554_gpio_uninit(client); +fail4: media_entity_cleanup(&flash->sd.entity); +fail3: v4l2_ctrl_handler_free(&flash->ctrl_handler); fail2: v4l2_device_unregister_subdev(&flash->sd);
There were several issues with handling errors in lm3554_probe(): - Probe did not set the error code when v4l2_ctrl_handler_init() failed. - It intermixed gotos for handling errors of v4l2_ctrl_handler_init() and media_entity_pads_init(). - Probe did not free resources in case of failures of atomisp_register_i2c_module(). The patch fixes all these issues. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Evgeny Novikov <novikov@ispras.ru> --- .../media/atomisp/i2c/atomisp-lm3554.c | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-)