* [PATCH] adp1653: check error code of adp1653_init_controls
@ 2011-07-27 7:58 Andy Shevchenko
2011-07-27 8:15 ` Sakari Ailus
0 siblings, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2011-07-27 7:58 UTC (permalink / raw)
To: linux-media, linux-kernel
Cc: Andy Shevchenko, Mauro Carvalho Chehab, Sakari Ailus
Potentially the adp1653_init_controls could return an error. In our case the
error was ignored, meanwhile it means incorrect initialization of V4L2
controls.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
---
drivers/media/video/adp1653.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
index 8ad89ff..3379e6d 100644
--- a/drivers/media/video/adp1653.c
+++ b/drivers/media/video/adp1653.c
@@ -429,7 +429,11 @@ static int adp1653_probe(struct i2c_client *client,
flash->subdev.internal_ops = &adp1653_internal_ops;
flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- adp1653_init_controls(flash);
+ ret = adp1653_init_controls(flash);
+ if (ret) {
+ kfree(flash);
+ return ret;
+ }
ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
if (ret < 0)
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] adp1653: check error code of adp1653_init_controls
2011-07-27 7:58 [PATCH] adp1653: check error code of adp1653_init_controls Andy Shevchenko
@ 2011-07-27 8:15 ` Sakari Ailus
2011-07-28 7:55 ` Andy Shevchenko
2011-07-28 7:59 ` [PATCHv2] " Andy Shevchenko
0 siblings, 2 replies; 6+ messages in thread
From: Sakari Ailus @ 2011-07-27 8:15 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-media, linux-kernel, Mauro Carvalho Chehab
On Wed, Jul 27, 2011 at 10:58:02AM +0300, Andy Shevchenko wrote:
> Potentially the adp1653_init_controls could return an error. In our case the
> error was ignored, meanwhile it means incorrect initialization of V4L2
> controls.
Hi, Andy!
Many thanks for the another patch! I'll add this to my next pull req as
well.
Just FYI: As this is clearly a regular patch for the V4L2 subsystem, I think
cc'ing the linux-kernel list isn't necessary.
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mauro Carvalho Chehab <mchehab@infradead.org>
> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> drivers/media/video/adp1653.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
> index 8ad89ff..3379e6d 100644
> --- a/drivers/media/video/adp1653.c
> +++ b/drivers/media/video/adp1653.c
> @@ -429,7 +429,11 @@ static int adp1653_probe(struct i2c_client *client,
> flash->subdev.internal_ops = &adp1653_internal_ops;
> flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> - adp1653_init_controls(flash);
> + ret = adp1653_init_controls(flash);
> + if (ret) {
> + kfree(flash);
> + return ret;
> + }
>
> ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
> if (ret < 0)
> --
> 1.7.5.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
--
Sakari Ailus
sakari.ailus@iki.fi
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] adp1653: check error code of adp1653_init_controls
2011-07-27 8:15 ` Sakari Ailus
@ 2011-07-28 7:55 ` Andy Shevchenko
2011-07-28 7:59 ` [PATCHv2] " Andy Shevchenko
1 sibling, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2011-07-28 7:55 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media
On Wed, 2011-07-27 at 11:15 +0300, Sakari Ailus wrote:
> On Wed, Jul 27, 2011 at 10:58:02AM +0300, Andy Shevchenko wrote:
> > Potentially the adp1653_init_controls could return an error. In our case the
> > error was ignored, meanwhile it means incorrect initialization of V4L2
> > controls.
>
> Hi, Andy!
>
> Many thanks for the another patch! I'll add this to my next pull req as
> well.
Please, skip this version, I will send updated one.
> Just FYI: As this is clearly a regular patch for the V4L2 subsystem, I think
> cc'ing the linux-kernel list isn't necessary.
Sure.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCHv2] adp1653: check error code of adp1653_init_controls
2011-07-27 8:15 ` Sakari Ailus
2011-07-28 7:55 ` Andy Shevchenko
@ 2011-07-28 7:59 ` Andy Shevchenko
2011-07-28 14:34 ` Laurent Pinchart
1 sibling, 1 reply; 6+ messages in thread
From: Andy Shevchenko @ 2011-07-28 7:59 UTC (permalink / raw)
To: linux-media; +Cc: Andy Shevchenko, Sakari Ailus
Potentially the adp1653_init_controls could return an error. In our case the
error was ignored, meanwhile it means incorrect initialization of V4L2
controls. Additionally we have to free control handler structures in case of
apd1653_init_controls or media_entity_init failure.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Sakari Ailus <sakari.ailus@iki.fi>
---
drivers/media/video/adp1653.c | 11 +++++++++--
1 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
index 8ad89ff..279d75d 100644
--- a/drivers/media/video/adp1653.c
+++ b/drivers/media/video/adp1653.c
@@ -429,12 +429,19 @@ static int adp1653_probe(struct i2c_client *client,
flash->subdev.internal_ops = &adp1653_internal_ops;
flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
- adp1653_init_controls(flash);
+ ret = adp1653_init_controls(flash);
+ if (ret)
+ goto free_and_quit;
ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
if (ret < 0)
- kfree(flash);
+ goto free_and_quit;
+ return 0;
+
+free_and_quit:
+ v4l2_ctrl_handler_free(&flash->ctrls);
+ kfree(flash);
return ret;
}
--
1.7.5.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv2] adp1653: check error code of adp1653_init_controls
2011-07-28 7:59 ` [PATCHv2] " Andy Shevchenko
@ 2011-07-28 14:34 ` Laurent Pinchart
2011-07-29 7:10 ` Andy Shevchenko
0 siblings, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2011-07-28 14:34 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-media, Sakari Ailus
Hi Andy,
On Thursday 28 July 2011 09:59:38 Andy Shevchenko wrote:
> Potentially the adp1653_init_controls could return an error. In our case
> the error was ignored, meanwhile it means incorrect initialization of V4L2
> controls. Additionally we have to free control handler structures in case
> of apd1653_init_controls or media_entity_init failure.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Sakari Ailus <sakari.ailus@iki.fi>
> ---
> drivers/media/video/adp1653.c | 11 +++++++++--
> 1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/video/adp1653.c b/drivers/media/video/adp1653.c
> index 8ad89ff..279d75d 100644
> --- a/drivers/media/video/adp1653.c
> +++ b/drivers/media/video/adp1653.c
> @@ -429,12 +429,19 @@ static int adp1653_probe(struct i2c_client *client,
> flash->subdev.internal_ops = &adp1653_internal_ops;
> flash->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
>
> - adp1653_init_controls(flash);
> + ret = adp1653_init_controls(flash);
> + if (ret)
> + goto free_and_quit;
>
> ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
> if (ret < 0)
> - kfree(flash);
> + goto free_and_quit;
>
> + return 0;
> +
> +free_and_quit:
> + v4l2_ctrl_handler_free(&flash->ctrls);
> + kfree(flash);
> return ret;
What about
ret = adp1653_init_controls(flash);
if (ret)
goto done;
ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
done:
if (ret < 0) {
v4l2_ctrl_handler_free(&flash->ctrls);
kfree(flash);
}
return ret;
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCHv2] adp1653: check error code of adp1653_init_controls
2011-07-28 14:34 ` Laurent Pinchart
@ 2011-07-29 7:10 ` Andy Shevchenko
0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2011-07-29 7:10 UTC (permalink / raw)
To: Laurent Pinchart
On Thu, 2011-07-28 at 16:34 +0200, Laurent Pinchart wrote:
> > - adp1653_init_controls(flash);
> > + ret = adp1653_init_controls(flash);
> > + if (ret)
> > + goto free_and_quit;
> >
> > ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
> > if (ret < 0)
> > - kfree(flash);
> > + goto free_and_quit;
> >
> > + return 0;
> > +
> > +free_and_quit:
> > + v4l2_ctrl_handler_free(&flash->ctrls);
> > + kfree(flash);
> > return ret;
> What about
> ret = adp1653_init_controls(flash);
> if (ret)
> goto done;
>
> ret = media_entity_init(&flash->subdev.entity, 0, NULL, 0);
>
> done:
> if (ret < 0) {
> v4l2_ctrl_handler_free(&flash->ctrls);
> kfree(flash);
> }
>
> return ret;
There is no difference at first glance. However, your variant is less
straight to understand for my opinion.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-07-29 7:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-27 7:58 [PATCH] adp1653: check error code of adp1653_init_controls Andy Shevchenko
2011-07-27 8:15 ` Sakari Ailus
2011-07-28 7:55 ` Andy Shevchenko
2011-07-28 7:59 ` [PATCHv2] " Andy Shevchenko
2011-07-28 14:34 ` Laurent Pinchart
2011-07-29 7:10 ` Andy Shevchenko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox