* [PATCH v2 media-next] media: rkisp1: Fix unused value issue
@ 2024-11-18 9:37 Dheeraj Reddy Jonnalagadda
2024-11-18 10:18 ` Jacopo Mondi
2024-11-18 11:12 ` Laurent Pinchart
0 siblings, 2 replies; 4+ messages in thread
From: Dheeraj Reddy Jonnalagadda @ 2024-11-18 9:37 UTC (permalink / raw)
To: dafna, laurent.pinchart, linux-media
Cc: mchehab, heiko, linux-rockchip, linux-arm-kernel, linux-kernel,
Dheeraj Reddy Jonnalagadda
This commit fixes an unused value issue detected by Coverity (CID
1519008). If ret is set to an error value in the switch statement, it is
not handled before being overwritten later.
Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
---
drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
index dd114ab77800..9ad5026ab10a 100644
--- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
+++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
@@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
break;
}
+ if (ret)
+ break;
+
/* Parse the endpoint and validate the bus type. */
ret = v4l2_fwnode_endpoint_parse(ep, &vep);
if (ret) {
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2 media-next] media: rkisp1: Fix unused value issue
2024-11-18 9:37 [PATCH v2 media-next] media: rkisp1: Fix unused value issue Dheeraj Reddy Jonnalagadda
@ 2024-11-18 10:18 ` Jacopo Mondi
2024-11-19 5:54 ` Dheeraj Reddy Jonnalagadda
2024-11-18 11:12 ` Laurent Pinchart
1 sibling, 1 reply; 4+ messages in thread
From: Jacopo Mondi @ 2024-11-18 10:18 UTC (permalink / raw)
To: Dheeraj Reddy Jonnalagadda
Cc: dafna, laurent.pinchart, linux-media, mchehab, heiko,
linux-rockchip, linux-arm-kernel, linux-kernel
Hi Dheeraj
On Mon, Nov 18, 2024 at 03:07:21PM +0530, Dheeraj Reddy Jonnalagadda wrote:
> This commit fixes an unused value issue detected by Coverity (CID
> 1519008). If ret is set to an error value in the switch statement, it is
> not handled before being overwritten later.
>
> Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
Indeed there's something fishy here, however the issue is not very
much about ret being overritten but rather the error condition
fwnode_graph_for_each_endpoint(fwnode, ep) {
switch (reg) {
case 0:
HERE ----> (!rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
ret = -EINVAL;
break;
}
break;
case 1:
vep.bus_type = V4L2_MBUS_UNKNOWN;
break;
}
}
breaks the inner switch and not the for loop.
I would
1) Slight reword the commit message to make it about missing an error
condition
2) Add a Fixes tag
Fixes: 7d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")
so that this is collected in the stable trees
> ---
> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index dd114ab77800..9ad5026ab10a 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
> break;
> }
>
> + if (ret)
> + break;
> +
The change is correct
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Thanks
j
> /* Parse the endpoint and validate the bus type. */
> ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> if (ret) {
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2 media-next] media: rkisp1: Fix unused value issue
2024-11-18 10:18 ` Jacopo Mondi
@ 2024-11-19 5:54 ` Dheeraj Reddy Jonnalagadda
0 siblings, 0 replies; 4+ messages in thread
From: Dheeraj Reddy Jonnalagadda @ 2024-11-19 5:54 UTC (permalink / raw)
To: Jacopo Mondi
Cc: dafna, laurent.pinchart, linux-media, mchehab, heiko,
linux-rockchip, linux-arm-kernel, linux-kernel
On Mon, Nov 18, 2024 at 11:18:34AM +0100, Jacopo Mondi wrote:
> Hi Dheeraj
>
> On Mon, Nov 18, 2024 at 03:07:21PM +0530, Dheeraj Reddy Jonnalagadda wrote:
> > This commit fixes an unused value issue detected by Coverity (CID
> > 1519008). If ret is set to an error value in the switch statement, it is
> > not handled before being overwritten later.
> >
> > Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
>
> Indeed there's something fishy here, however the issue is not very
> much about ret being overritten but rather the error condition
>
> fwnode_graph_for_each_endpoint(fwnode, ep) {
> switch (reg) {
> case 0:
> HERE ----> (!rkisp1_has_feature(rkisp1, MIPI_CSI2)) {
> ret = -EINVAL;
> break;
> }
>
> break;
>
> case 1:
> vep.bus_type = V4L2_MBUS_UNKNOWN;
> break;
> }
> }
>
> breaks the inner switch and not the for loop.
>
> I would
> 1) Slight reword the commit message to make it about missing an error
> condition
> 2) Add a Fixes tag
> Fixes: 7d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")
> so that this is collected in the stable trees
>
> > ---
> > drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > index dd114ab77800..9ad5026ab10a 100644
> > --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> > @@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
> > break;
> > }
> >
> > + if (ret)
> > + break;
> > +
>
> The change is correct
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
> j
>
> > /* Parse the endpoint and validate the bus type. */
> > ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> > if (ret) {
> > --
> > 2.34.1
> >
> >
Hi Jacopo,
Thank you for your feedback. I agree with your suggestion and will update
the commit message to describe the missing error condition.
-Dheeraj
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 media-next] media: rkisp1: Fix unused value issue
2024-11-18 9:37 [PATCH v2 media-next] media: rkisp1: Fix unused value issue Dheeraj Reddy Jonnalagadda
2024-11-18 10:18 ` Jacopo Mondi
@ 2024-11-18 11:12 ` Laurent Pinchart
1 sibling, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2024-11-18 11:12 UTC (permalink / raw)
To: Dheeraj Reddy Jonnalagadda
Cc: dafna, linux-media, mchehab, heiko, linux-rockchip,
linux-arm-kernel, linux-kernel
Hi Dheeraj,
Thank you for the patch.
On Mon, Nov 18, 2024 at 03:07:21PM +0530, Dheeraj Reddy Jonnalagadda wrote:
> This commit fixes an unused value issue detected by Coverity (CID
> 1519008). If ret is set to an error value in the switch statement, it is
> not handled before being overwritten later.
>
> Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> index dd114ab77800..9ad5026ab10a 100644
> --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c
> @@ -228,6 +228,9 @@ static int rkisp1_subdev_notifier_register(struct rkisp1_device *rkisp1)
> break;
> }
>
> + if (ret)
> + break;
> +
> /* Parse the endpoint and validate the bus type. */
> ret = v4l2_fwnode_endpoint_parse(ep, &vep);
> if (ret) {
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-11-19 5:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-11-18 9:37 [PATCH v2 media-next] media: rkisp1: Fix unused value issue Dheeraj Reddy Jonnalagadda
2024-11-18 10:18 ` Jacopo Mondi
2024-11-19 5:54 ` Dheeraj Reddy Jonnalagadda
2024-11-18 11:12 ` Laurent Pinchart
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).