Hi Dan, thanks for noticing this, On Tue, Jul 31, 2018 at 09:35:54PM +0300, Dan Carpenter wrote: > Hello Jacopo Mondi, > > The patch aab7ed1c3927: "media: i2c: Add driver for Aptina MT9V111" > from Jul 25, 2018, leads to the following static checker warning: > > drivers/media/i2c/mt9v111.c:1163 mt9v111_probe() warn: passing zero to 'PTR_ERR' > drivers/media/i2c/mt9v111.c:1173 mt9v111_probe() warn: passing zero to 'PTR_ERR' > drivers/media/i2c/mt9v111.c:1184 mt9v111_probe() warn: passing zero to 'PTR_ERR' > drivers/media/i2c/mt9v111.c:1194 mt9v111_probe() warn: passing zero to 'PTR_ERR' > > drivers/media/i2c/mt9v111.c > 1155 v4l2_ctrl_handler_init(&mt9v111->ctrls, 5); > 1156 > 1157 mt9v111->auto_awb = v4l2_ctrl_new_std(&mt9v111->ctrls, > 1158 &mt9v111_ctrl_ops, > 1159 V4L2_CID_AUTO_WHITE_BALANCE, > 1160 0, 1, 1, > 1161 V4L2_WHITE_BALANCE_AUTO); > 1162 if (IS_ERR_OR_NULL(mt9v111->auto_awb)) { > 1163 ret = PTR_ERR(mt9v111->auto_awb); > > This just returns success because v4l2_ctrl_new_std() only return NULL Correct, sorry, I didn't notice that. > on error, it never returns error pointers. I guess we should set ret to > EINVAL? > > if (!mt9v111->auto_awb) { > ret = -EINVAL; > goto error_free_ctrls; > } > We can do even better than that. The v4l2 control handler retains errors in a flag I can inspect after having added/created all controls here and here below. I can return that error flag if something goes wrong. Thanks j > 1164 goto error_free_ctrls; > 1165 } > 1166 > 1167 mt9v111->auto_exp = v4l2_ctrl_new_std_menu(&mt9v111->ctrls, > 1168 &mt9v111_ctrl_ops, > 1169 V4L2_CID_EXPOSURE_AUTO, > 1170 V4L2_EXPOSURE_MANUAL, > 1171 0, V4L2_EXPOSURE_AUTO); > 1172 if (IS_ERR_OR_NULL(mt9v111->auto_exp)) { > 1173 ret = PTR_ERR(mt9v111->auto_exp); > 1174 goto error_free_ctrls; > 1175 } > 1176 > 1177 /* Initialize timings */ > 1178 mt9v111->hblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops, > 1179 V4L2_CID_HBLANK, > 1180 MT9V111_CORE_R05_MIN_HBLANK, > 1181 MT9V111_CORE_R05_MAX_HBLANK, 1, > 1182 MT9V111_CORE_R05_DEF_HBLANK); > 1183 if (IS_ERR_OR_NULL(mt9v111->hblank)) { > 1184 ret = PTR_ERR(mt9v111->hblank); > 1185 goto error_free_ctrls; > 1186 } > 1187 > 1188 mt9v111->vblank = v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops, > 1189 V4L2_CID_VBLANK, > 1190 MT9V111_CORE_R06_MIN_VBLANK, > 1191 MT9V111_CORE_R06_MAX_VBLANK, 1, > 1192 MT9V111_CORE_R06_DEF_VBLANK); > 1193 if (IS_ERR_OR_NULL(mt9v111->vblank)) { > 1194 ret = PTR_ERR(mt9v111->vblank); > 1195 goto error_free_ctrls; > 1196 } > 1197 > 1198 /* PIXEL_RATE is fixed: just expose it to user space. */ > 1199 v4l2_ctrl_new_std(&mt9v111->ctrls, &mt9v111_ctrl_ops, > > regards, > dan carpenter
Attachment:
signature.asc
Description: PGP signature