From: Alex Dewar <alex.dewar90@gmail.com> To: Mauro Carvalho Chehab <mchehab@kernel.org>, Sakari Ailus <sakari.ailus@linux.intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Alex Dewar <alex.dewar90@gmail.com>, Dan Carpenter <dan.carpenter@oracle.com>, Alan Cox <alan@linux.intel.com>, linux-media@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: [PATCH REBASE 1/3] staging: media: atomisp: Fix error path in lm3554_probe() Date: Tue, 22 Sep 2020 10:09:08 +0100 [thread overview] Message-ID: <20200922090914.20702-2-alex.dewar90@gmail.com> (raw) In-Reply-To: <20200922090914.20702-1-alex.dewar90@gmail.com> The error path for lm3554_probe() contains a number of bugs, including: * resource leaks * jumping to error labels out of sequence * not setting the return value appropriately Fix it up and give the labels more memorable names. This issue has existed since the code was originally contributed in commit a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2"), although the code was subsequently removed altogether and then reinstated with commit ad85094b293e ("Revert "media: staging: atomisp: Remove driver""). Addresses-Coverity: 1496802 ("Resource leaks") Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2") Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> --- .../media/atomisp/i2c/atomisp-lm3554.c | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c index 809010af7855..67e62b96adf9 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c @@ -847,7 +847,6 @@ static void *lm3554_platform_data_func(struct i2c_client *client) static int lm3554_probe(struct i2c_client *client) { - int err = 0; struct lm3554 *flash; unsigned int i; int ret; @@ -863,28 +862,28 @@ 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 = - v4l2_ctrl_handler_init(&flash->ctrl_handler, - ARRAY_SIZE(lm3554_controls)); + ret = v4l2_ctrl_handler_init(&flash->ctrl_handler, + ARRAY_SIZE(lm3554_controls)); if (ret) { - dev_err(&client->dev, "error initialize a ctrl_handler.\n"); - goto fail2; + dev_err(&client->dev, "error initializing ctrl_handler"); + goto err_unregister_sd; } for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++) v4l2_ctrl_new_custom(&flash->ctrl_handler, &lm3554_controls[i], NULL); - if (flash->ctrl_handler.error) { - dev_err(&client->dev, "ctrl_handler error.\n"); - goto fail2; + ret = flash->ctrl_handler.error; + if (ret) { + dev_err(&client->dev, "ctrl_handler error"); + goto err_free_ctrl_handler; } flash->sd.ctrl_handler = &flash->ctrl_handler; - err = media_entity_pads_init(&flash->sd.entity, 0, NULL); - if (err) { - dev_err(&client->dev, "error initialize a media entity.\n"); - goto fail1; + ret = media_entity_pads_init(&flash->sd.entity, 0, NULL); + if (ret) { + dev_err(&client->dev, "error initializing media entity"); + goto err_free_ctrl_handler; } flash->sd.entity.function = MEDIA_ENT_F_FLASH; @@ -893,20 +892,27 @@ static int lm3554_probe(struct i2c_client *client) timer_setup(&flash->flash_off_delay, lm3554_flash_off_delay, 0); - err = lm3554_gpio_init(client); - if (err) { + ret = lm3554_gpio_init(client); + if (ret) { dev_err(&client->dev, "gpio request/direction_output fail"); - goto fail2; + goto err_del_timer; } - return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); -fail2: + + ret = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); + if (ret) + return ret; + + return 0; + +err_del_timer: + del_timer_sync(&flash->flash_off_delay); media_entity_cleanup(&flash->sd.entity); +err_free_ctrl_handler: v4l2_ctrl_handler_free(&flash->ctrl_handler); -fail1: +err_unregister_sd: v4l2_device_unregister_subdev(&flash->sd); kfree(flash); - - return err; + return ret; } static int lm3554_remove(struct i2c_client *client) -- 2.28.0
WARNING: multiple messages have this Message-ID (diff)
From: Alex Dewar <alex.dewar90@gmail.com> To: Mauro Carvalho Chehab <mchehab@kernel.org>, Sakari Ailus <sakari.ailus@linux.intel.com>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Alex Dewar <alex.dewar90@gmail.com>, Dan Carpenter <dan.carpenter@oracle.com>, Alan Cox <alan@linux.intel.com>, linux-media@vger.kernel.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: [PATCH REBASE 1/3] staging: media: atomisp: Fix error path in lm3554_probe() Date: Tue, 22 Sep 2020 10:09:08 +0100 [thread overview] Message-ID: <20200922090914.20702-2-alex.dewar90@gmail.com> (raw) In-Reply-To: <20200922090914.20702-1-alex.dewar90@gmail.com> The error path for lm3554_probe() contains a number of bugs, including: * resource leaks * jumping to error labels out of sequence * not setting the return value appropriately Fix it up and give the labels more memorable names. This issue has existed since the code was originally contributed in commit a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2"), although the code was subsequently removed altogether and then reinstated with commit ad85094b293e ("Revert "media: staging: atomisp: Remove driver""). Addresses-Coverity: 1496802 ("Resource leaks") Fixes: a49d25364dfb ("staging/atomisp: Add support for the Intel IPU v2") Signed-off-by: Alex Dewar <alex.dewar90@gmail.com> Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com> --- .../media/atomisp/i2c/atomisp-lm3554.c | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c index 809010af7855..67e62b96adf9 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-lm3554.c @@ -847,7 +847,6 @@ static void *lm3554_platform_data_func(struct i2c_client *client) static int lm3554_probe(struct i2c_client *client) { - int err = 0; struct lm3554 *flash; unsigned int i; int ret; @@ -863,28 +862,28 @@ 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 = - v4l2_ctrl_handler_init(&flash->ctrl_handler, - ARRAY_SIZE(lm3554_controls)); + ret = v4l2_ctrl_handler_init(&flash->ctrl_handler, + ARRAY_SIZE(lm3554_controls)); if (ret) { - dev_err(&client->dev, "error initialize a ctrl_handler.\n"); - goto fail2; + dev_err(&client->dev, "error initializing ctrl_handler"); + goto err_unregister_sd; } for (i = 0; i < ARRAY_SIZE(lm3554_controls); i++) v4l2_ctrl_new_custom(&flash->ctrl_handler, &lm3554_controls[i], NULL); - if (flash->ctrl_handler.error) { - dev_err(&client->dev, "ctrl_handler error.\n"); - goto fail2; + ret = flash->ctrl_handler.error; + if (ret) { + dev_err(&client->dev, "ctrl_handler error"); + goto err_free_ctrl_handler; } flash->sd.ctrl_handler = &flash->ctrl_handler; - err = media_entity_pads_init(&flash->sd.entity, 0, NULL); - if (err) { - dev_err(&client->dev, "error initialize a media entity.\n"); - goto fail1; + ret = media_entity_pads_init(&flash->sd.entity, 0, NULL); + if (ret) { + dev_err(&client->dev, "error initializing media entity"); + goto err_free_ctrl_handler; } flash->sd.entity.function = MEDIA_ENT_F_FLASH; @@ -893,20 +892,27 @@ static int lm3554_probe(struct i2c_client *client) timer_setup(&flash->flash_off_delay, lm3554_flash_off_delay, 0); - err = lm3554_gpio_init(client); - if (err) { + ret = lm3554_gpio_init(client); + if (ret) { dev_err(&client->dev, "gpio request/direction_output fail"); - goto fail2; + goto err_del_timer; } - return atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); -fail2: + + ret = atomisp_register_i2c_module(&flash->sd, NULL, LED_FLASH); + if (ret) + return ret; + + return 0; + +err_del_timer: + del_timer_sync(&flash->flash_off_delay); media_entity_cleanup(&flash->sd.entity); +err_free_ctrl_handler: v4l2_ctrl_handler_free(&flash->ctrl_handler); -fail1: +err_unregister_sd: v4l2_device_unregister_subdev(&flash->sd); kfree(flash); - - return err; + return ret; } static int lm3554_remove(struct i2c_client *client) -- 2.28.0 _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
next prev parent reply other threads:[~2020-09-22 9:09 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-21 21:53 [PATCH RESEND 0/5] atomisp: Fixes and cleanups Alex Dewar 2020-09-21 21:53 ` Alex Dewar 2020-09-21 21:53 ` [PATCH RESEND 1/5] staging: media: atomisp: Fix error path in lm3554_probe() Alex Dewar 2020-09-21 21:53 ` Alex Dewar 2020-09-21 21:53 ` [PATCH RESEND 2/5] staging: media: atomisp: Remove unhelpful info message Alex Dewar 2020-09-21 21:53 ` Alex Dewar 2020-09-21 21:53 ` [PATCH RESEND 3/5] staging: media: atomisp: Don't do unnecessary zeroing of memory Alex Dewar 2020-09-21 21:53 ` Alex Dewar 2020-09-21 21:53 ` [PATCH RESEND 4/5] staging: media: atomisp: Don't abort on error in module exit path Alex Dewar 2020-09-21 21:53 ` Alex Dewar 2020-09-21 21:53 ` [PATCH RESEND 5/5] staging: media: atomisp: Fix bool-related style issues Alex Dewar 2020-09-21 21:53 ` Alex Dewar 2020-09-22 8:11 ` [PATCH RESEND 0/5] atomisp: Fixes and cleanups Mauro Carvalho Chehab 2020-09-22 8:11 ` Mauro Carvalho Chehab 2020-09-22 8:16 ` Alex Dewar 2020-09-22 8:16 ` Alex Dewar 2020-09-22 9:09 ` [PATCH REBASE 0/3] atomisp: Rebased fixes Alex Dewar 2020-09-22 9:09 ` Alex Dewar 2020-09-22 9:09 ` Alex Dewar [this message] 2020-09-22 9:09 ` [PATCH REBASE 1/3] staging: media: atomisp: Fix error path in lm3554_probe() Alex Dewar 2020-09-22 9:09 ` [PATCH REBASE 2/3] staging: media: atomisp: Remove unhelpful info message Alex Dewar 2020-09-22 9:09 ` Alex Dewar 2020-09-22 9:09 ` [PATCH REBASE 3/3] staging: media: atomisp: Don't abort on error in module exit path Alex Dewar 2020-09-22 9:09 ` Alex Dewar 2020-09-22 9:27 ` [PATCH REBASE 0/3] atomisp: Rebased fixes Mauro Carvalho Chehab 2020-09-22 9:27 ` Mauro Carvalho Chehab 2020-09-22 11:02 ` Alex Dewar 2020-09-22 11:02 ` Alex Dewar 2020-09-22 13:27 ` Dan Carpenter 2020-09-22 13:27 ` Dan Carpenter 2020-09-22 13:51 ` Alex Dewar 2020-09-22 13:51 ` Alex Dewar
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200922090914.20702-2-alex.dewar90@gmail.com \ --to=alex.dewar90@gmail.com \ --cc=alan@linux.intel.com \ --cc=dan.carpenter@oracle.com \ --cc=devel@driverdev.osuosl.org \ --cc=gregkh@linuxfoundation.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-media@vger.kernel.org \ --cc=mchehab@kernel.org \ --cc=sakari.ailus@linux.intel.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.