| Message ID | 20241119072653.72260-1-dheeraj.linuxdev@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,media-next] media: rkisp1: Fix unused value issue | expand |
Hi Dheeraj, Thank you for the patch. On Tue, Nov 19, 2024 at 12:56:53PM +0530, Dheeraj Reddy Jonnalagadda wrote: > This commit fixes an unused value issue detected by Coverity (CID > 1519008). The error condition for the invalid MIPI CSI-2 is not > properly handled as the break statement would only exit the switch block > and not the entire loop. Fixed this by returning the error immediately > after the switch block. The patch doesn't "return immediately". You can write "Fix this by breaking from the look immediately after the switch block when an error occurs." or something similar. > > 'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver > optional")' The Fixes tag should be formatted on a single line, without outer quotes, and without a blank line between it and the Signed-off-by line: Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional") > Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com> I can update the commit message when applying the patch, there's no need to submit a v5, unless if you want to. Please let me know if I should take this version and update the commit message, or if you will send a v5. 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) {
HI Dheeraj On Tue, Nov 19, 2024 at 09:59:44AM +0200, Laurent Pinchart wrote: > Hi Dheeraj, > > Thank you for the patch. > > On Tue, Nov 19, 2024 at 12:56:53PM +0530, Dheeraj Reddy Jonnalagadda wrote: > > This commit fixes an unused value issue detected by Coverity (CID > > 1519008). The error condition for the invalid MIPI CSI-2 is not > > properly handled as the break statement would only exit the switch block > > and not the entire loop. Fixed this by returning the error immediately > > after the switch block. > > The patch doesn't "return immediately". You can write "Fix this by > breaking from the look immediately after the switch block when an error > occurs." or something similar. > > > > > 'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver > > optional")' > > The Fixes tag should be formatted on a single line, without outer > quotes, and without a blank line between it and the Signed-off-by line: > > Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional") > > > Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com> > > I can update the commit message when applying the patch, there's no need > to submit a v5, unless if you want to. Please let me know if I should > take this version and update the commit message, or if you will send a > v5. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> And when you receive a tag in a previous patch version, please carry it over to the next version you'll send ;) Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com> Thanks j > > > --- > > 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 >
On Tue, Nov 19, 2024 at 09:59:44AM +0200, Laurent Pinchart wrote: > Hi Dheeraj, > > Thank you for the patch. > > On Tue, Nov 19, 2024 at 12:56:53PM +0530, Dheeraj Reddy Jonnalagadda wrote: > > This commit fixes an unused value issue detected by Coverity (CID > > 1519008). The error condition for the invalid MIPI CSI-2 is not > > properly handled as the break statement would only exit the switch block > > and not the entire loop. Fixed this by returning the error immediately > > after the switch block. > > The patch doesn't "return immediately". You can write "Fix this by > breaking from the look immediately after the switch block when an error > occurs." or something similar. > > > > > 'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver > > optional")' > > The Fixes tag should be formatted on a single line, without outer > quotes, and without a blank line between it and the Signed-off-by line: > > Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional") > > > Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com> > > I can update the commit message when applying the patch, there's no need > to submit a v5, unless if you want to. Please let me know if I should > take this version and update the commit message, or if you will send a > v5. > > 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 Hi Laurent, Please go ahead and take this version and apply the patch with the updated commit message. Thank you and Jacopo for your valuable comments. As a new contributor, your feedback is extremely helpful. -Dheeraj
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) {
This commit fixes an unused value issue detected by Coverity (CID 1519008). The error condition for the invalid MIPI CSI-2 is not properly handled as the break statement would only exit the switch block and not the entire loop. Fixed this by returning the error immediately after the switch block. 'Fixes: 8d4f126fde89 ("media: rkisp1: Make the internal CSI-2 receiver optional")' Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@gmail.com> --- drivers/media/platform/rockchip/rkisp1/rkisp1-dev.c | 3 +++ 1 file changed, 3 insertions(+)