* Coverity: mmal_setup_video_component(): Code maintainability issues
@ 2020-04-14 15:33 coverity-bot
2020-04-14 16:49 ` Stefan Wahren
0 siblings, 1 reply; 3+ messages in thread
From: coverity-bot @ 2020-04-14 15:33 UTC (permalink / raw)
To: Stefan Wahren
Cc: Nicolas Saenz Julienne, Greg Kroah-Hartman, Gustavo A. R. Silva,
linux-next
Hello!
This is an experimental semi-automated report about issues detected by
Coverity from a scan of next-20200414 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan
You're getting this email because you were associated with the identified
lines of code (noted below) that were touched by commits:
Sun Mar 29 14:44:58 2020 +0200
1a59532382a6 ("staging: bcm2835-camera: Move video component setup in its own function")
Coverity reported the following:
*** CID 1492591: Code maintainability issues (UNUSED_VALUE)
/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c: 1014 in mmal_setup_video_component()
1008 if (overlay_enabled) {
1009 /* Need to disable the overlay before we can update
1010 * the resolution
1011 */
1012 ret = vchiq_mmal_port_disable(dev->instance, preview_port);
1013 if (!ret) {
vvv CID 1492591: Code maintainability issues (UNUSED_VALUE)
vvv Assigning value from "vchiq_mmal_port_connect_tunnel(dev->instance, preview_port, NULL)" to "ret" here, but that stored value is overwritten before it can be used.
1014 ret = vchiq_mmal_port_connect_tunnel(dev->instance,
1015 preview_port,
1016 NULL);
1017 }
1018 }
1019 preview_port->es.video.width = f->fmt.pix.width;
If this is a false positive, please let us know so we can mark it as
such, or teach the Coverity rules to be smarter. If not, please make
sure fixes get into linux-next. :) For patches fixing this, please
include these lines (but double-check the "Fixes" first):
Reported-by: coverity-bot <keescook+coverity-bot@chromium•org>
Addresses-Coverity-ID: 1492591 ("Code maintainability issues")
Fixes: 1a59532382a6 ("staging: bcm2835-camera: Move video component setup in its own function")
Thanks for your attention!
--
Coverity-bot
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Coverity: mmal_setup_video_component(): Code maintainability issues
2020-04-14 15:33 Coverity: mmal_setup_video_component(): Code maintainability issues coverity-bot
@ 2020-04-14 16:49 ` Stefan Wahren
2020-04-16 10:18 ` Dave Stevenson
0 siblings, 1 reply; 3+ messages in thread
From: Stefan Wahren @ 2020-04-14 16:49 UTC (permalink / raw)
To: coverity-bot, Dave Stevenson
Cc: Nicolas Saenz Julienne, Greg Kroah-Hartman, Gustavo A. R. Silva,
linux-next
Hi,
Am 14.04.20 um 17:33 schrieb coverity-bot:
> Hello!
>
> This is an experimental semi-automated report about issues detected by
> Coverity from a scan of next-20200414 as part of the linux-next scan project:
> https://scan.coverity.com/projects/linux-next-weekly-scan
>
> You're getting this email because you were associated with the identified
> lines of code (noted below) that were touched by commits:
>
> Sun Mar 29 14:44:58 2020 +0200
> 1a59532382a6 ("staging: bcm2835-camera: Move video component setup in its own function")
>
> Coverity reported the following:
>
> *** CID 1492591: Code maintainability issues (UNUSED_VALUE)
> /drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c: 1014 in mmal_setup_video_component()
> 1008 if (overlay_enabled) {
> 1009 /* Need to disable the overlay before we can update
> 1010 * the resolution
> 1011 */
> 1012 ret = vchiq_mmal_port_disable(dev->instance, preview_port);
> 1013 if (!ret) {
> vvv CID 1492591: Code maintainability issues (UNUSED_VALUE)
> vvv Assigning value from "vchiq_mmal_port_connect_tunnel(dev->instance, preview_port, NULL)" to "ret" here, but that stored value is overwritten before it can be used.
> 1014 ret = vchiq_mmal_port_connect_tunnel(dev->instance,
> 1015 preview_port,
> 1016 NULL);
> 1017 }
> 1018 }
> 1019 preview_port->es.video.width = f->fmt.pix.width;
>
> If this is a false positive, please let us know so we can mark it as
> such, or teach the Coverity rules to be smarter. If not, please make
> sure fixes get into linux-next. :) For patches fixing this, please
> include these lines (but double-check the "Fixes" first):
thanks for the report. The finding is correct, but the issue already
exists before. The intention of my patch was to increase readibility,
not to change the behavior.
My problem is that i'm not aware how to handle the error case here.
@Dave Should we bail out or ignore the error?
Best regards
Stefan
>
> Reported-by: coverity-bot <keescook+coverity-bot@chromium•org>
> Addresses-Coverity-ID: 1492591 ("Code maintainability issues")
> Fixes: 1a59532382a6 ("staging: bcm2835-camera: Move video component setup in its own function")
>
> Thanks for your attention!
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: Coverity: mmal_setup_video_component(): Code maintainability issues
2020-04-14 16:49 ` Stefan Wahren
@ 2020-04-16 10:18 ` Dave Stevenson
0 siblings, 0 replies; 3+ messages in thread
From: Dave Stevenson @ 2020-04-16 10:18 UTC (permalink / raw)
To: Stefan Wahren
Cc: coverity-bot, Nicolas Saenz Julienne, Greg Kroah-Hartman,
Gustavo A. R. Silva, linux-next
Hi Stefan
On Tue, 14 Apr 2020 at 17:49, Stefan Wahren <stefan.wahren@i2se•com> wrote:
>
> Hi,
>
> Am 14.04.20 um 17:33 schrieb coverity-bot:
> > Hello!
> >
> > This is an experimental semi-automated report about issues detected by
> > Coverity from a scan of next-20200414 as part of the linux-next scan project:
> > https://scan.coverity.com/projects/linux-next-weekly-scan
> >
> > You're getting this email because you were associated with the identified
> > lines of code (noted below) that were touched by commits:
> >
> > Sun Mar 29 14:44:58 2020 +0200
> > 1a59532382a6 ("staging: bcm2835-camera: Move video component setup in its own function")
> >
> > Coverity reported the following:
> >
> > *** CID 1492591: Code maintainability issues (UNUSED_VALUE)
> > /drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c: 1014 in mmal_setup_video_component()
> > 1008 if (overlay_enabled) {
> > 1009 /* Need to disable the overlay before we can update
> > 1010 * the resolution
> > 1011 */
> > 1012 ret = vchiq_mmal_port_disable(dev->instance, preview_port);
> > 1013 if (!ret) {
> > vvv CID 1492591: Code maintainability issues (UNUSED_VALUE)
> > vvv Assigning value from "vchiq_mmal_port_connect_tunnel(dev->instance, preview_port, NULL)" to "ret" here, but that stored value is overwritten before it can be used.
> > 1014 ret = vchiq_mmal_port_connect_tunnel(dev->instance,
> > 1015 preview_port,
> > 1016 NULL);
> > 1017 }
> > 1018 }
> > 1019 preview_port->es.video.width = f->fmt.pix.width;
> >
> > If this is a false positive, please let us know so we can mark it as
> > such, or teach the Coverity rules to be smarter. If not, please make
> > sure fixes get into linux-next. :) For patches fixing this, please
> > include these lines (but double-check the "Fixes" first):
>
> thanks for the report. The finding is correct, but the issue already
> exists before. The intention of my patch was to increase readibility,
> not to change the behavior.
>
> My problem is that i'm not aware how to handle the error case here.
>
> @Dave Should we bail out or ignore the error?
Neither vchiq_mmal_port_disable nor
vchiq_mmal_port_connect_tunnel(inst, src, NULL) should fail - that
would imply something seriously wrong within the firmware.
I'd suggest ignore the error - if something is seriously wrong then
it'll fail and bail later on.
Looking at vchiq_mmal_port_connect_tunnel, if dst is NULL (as in this
case) then the first thing it does is call port_disable, same as
vchiq_mmal_port_disable. The call to vchiq_mmal_port_disable can
therefore be removed. Same in vidioc_overlay.
Dave
> Best regards
> Stefan
>
> >
> > Reported-by: coverity-bot <keescook+coverity-bot@chromium•org>
> > Addresses-Coverity-ID: 1492591 ("Code maintainability issues")
> > Fixes: 1a59532382a6 ("staging: bcm2835-camera: Move video component setup in its own function")
> >
> > Thanks for your attention!
> >
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-04-16 10:18 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-14 15:33 Coverity: mmal_setup_video_component(): Code maintainability issues coverity-bot
2020-04-14 16:49 ` Stefan Wahren
2020-04-16 10:18 ` Dave Stevenson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox