Coverity: imx290_ctrl_init(): Error handling issues

Hello!

This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20221110 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:

Thu Oct 27 14:38:02 2022 +0300

    4c9c93cf8657 ("media: i2c: imx290: Create controls for fwnode properties")

Coverity reported the following:

*** CID 1527251: Error handling issues (CHECKED_RETURN) drivers/media/i2c/imx290.c:1056 in imx290_ctrl_init()
1050 imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
1051 V4L2_CID_VBLANK, blank, blank, 1,
1052 blank);
1053 if (imx290->vblank)
1054 imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
1055
vvv CID 1527251: Error handling issues (CHECKED_RETURN)
vvv Calling "v4l2_ctrl_new_fwnode_properties" without checking return value (as is done elsewhere 9 out of 10 times).
1056 v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops,
1057 &props);
1058
1059 imx290->sd.ctrl_handler = &imx290->ctrls;
1060
1061 if (imx290->ctrls.error) {

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 <[email protected]>

Addresses-Coverity-ID: 1527251 ("Error handling issues")

Fixes 4c9c93cf8657 ("media: i2c: imx290: Create controls for fwnode properties")

Thanks for your attention!

--
Coverity-bot

Quoted 32 lines · coverity-bot

Hello!

This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20221110 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:

Thu Oct 27 14:38:02 2022 +0300

    4c9c93cf8657 ("media: i2c: imx290: Create controls for fwnode properties")

Coverity reported the following:

*** CID 1527251: Error handling issues (CHECKED_RETURN) drivers/media/i2c/imx290.c:1056 in imx290_ctrl_init()
1050 imx290->vblank = v4l2_ctrl_new_std(&imx290->ctrls, &imx290_ctrl_ops,
1051 V4L2_CID_VBLANK, blank, blank, 1,
1052 blank);
1053 if (imx290->vblank)
1054 imx290->vblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
1055
vvv CID 1527251: Error handling issues (CHECKED_RETURN)
vvv Calling "v4l2_ctrl_new_fwnode_properties" without checking return value (as is done elsewhere 9 out of 10 times).
1056 v4l2_ctrl_new_fwnode_properties(&imx290->ctrls, &imx290_ctrl_ops,
1057 &props);
1058
1059 imx290->sd.ctrl_handler = &imx290->ctrls;
1060
1061 if (imx290->ctrls.error) {

Quoted 4 lines

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):

I looked at this one when the patches were sent to the list.

On failure, v4l2_ctrl_new_fwnode_properties will have set the error flag in struct v4l2_ctrl_handler. This is also what it returns.

In most of the existing drivers the error flag has already been checked before calling v4l2_ctrl_new_fwnode_properties, therefore the return value has to be checked explicitly. In this case it is checked at line 1061 which is after v4l2_ctrl_new_fwnode_properties has been called, and therefore there is no need to check the return value of the call.

IMHO Neither is particularly right or wrong, just slightly different approaches. In some regards this new code pattern is nicer as it removes a number of error handling paths.