| Message ID | 20171211161058.6cdedb7a@vento.lan (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mauro, On Monday, 11 December 2017 20:10:58 EET Mauro Carvalho Chehab wrote: > Em Thu, 02 Nov 2017 04:51:40 +0200 Laurent Pinchart escreveu: > > On Wednesday, 1 November 2017 23:05:45 EET Mauro Carvalho Chehab wrote: > >> Smatch reports this warning: > >> drivers/media/v4l2-core/v4l2-async.c:597 v4l2_async_register_subdev() > >> > >> error: uninitialized symbol 'ret'. > >> > >> However, there's nothing wrong there. So, just shut up the > >> warning. > > > > Nothing wrong, really ? ret does seem to be used uninitialized when the > > function returns at the very last line. > > There's nothing wrong. If you follow the logic, you'll see that > the line: > > return ret; > > is called only at "err_unbind" label, with is called only on > two places: > > ret = v4l2_async_match_notify(notifier, v4l2_dev, sd, asd); > if (ret) > goto err_unbind; > > ret = v4l2_async_notifier_try_complete(notifier); > if (ret) > goto err_unbind; > > There, ret is defined. > > Yeah, the logic there is confusing. I had missed the return 0 just before the error label. Sorry for the noise.
Hi folks, On Tue, Dec 12, 2017 at 12:13:59AM +0200, Laurent Pinchart wrote: > Hi Mauro, > > On Monday, 11 December 2017 20:10:58 EET Mauro Carvalho Chehab wrote: > > Em Thu, 02 Nov 2017 04:51:40 +0200 Laurent Pinchart escreveu: > > > On Wednesday, 1 November 2017 23:05:45 EET Mauro Carvalho Chehab wrote: > > >> Smatch reports this warning: > > >> drivers/media/v4l2-core/v4l2-async.c:597 v4l2_async_register_subdev() > > >> > > >> error: uninitialized symbol 'ret'. > > >> > > >> However, there's nothing wrong there. So, just shut up the > > >> warning. > > > > > > Nothing wrong, really ? ret does seem to be used uninitialized when the > > > function returns at the very last line. > > > > There's nothing wrong. If you follow the logic, you'll see that > > the line: > > > > return ret; > > > > is called only at "err_unbind" label, with is called only on > > two places: > > > > ret = v4l2_async_match_notify(notifier, v4l2_dev, sd, asd); > > if (ret) > > goto err_unbind; > > > > ret = v4l2_async_notifier_try_complete(notifier); > > if (ret) > > goto err_unbind; > > > > There, ret is defined. > > > > Yeah, the logic there is confusing. > > I had missed the return 0 just before the error label. Sorry for the noise. I believe the matter has been addressed by this patch: commit 580db6ca62c168733c6fd338cd2f0ebf39135283 Author: Colin Ian King <colin.king@canonical.com> Date: Fri Nov 3 02:58:27 2017 -0400 media: v4l: async: fix return of unitialized variable ret A shadow declaration of variable ret is being assigned a return error status and this value is being lost when the error exit goto's jump out of the local scope. This leads to an uninitalized error return value in the outer scope being returned. Fix this by removing the inner scoped declaration of variable ret. Detected by CoverityScan, CID#1460380 ("Uninitialized scalar variable") Fixes: fb45f436b818 ("media: v4l: async: Fix notifier complete callback error handling") Signed-off-by: Colin Ian King <colin.king@canonical.com> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
--- patchwork.orig/drivers/media/v4l2-core/v4l2-async.c +++ patchwork/drivers/media/v4l2-core/v4l2-async.c @@ -532,7 +532,7 @@ int v4l2_async_register_subdev(struct v4 { struct v4l2_async_notifier *subdev_notifier; struct v4l2_async_notifier *notifier; - int ret; + int ret = 0; /* * No reference taken. The reference is held by the device @@ -560,11 +560,11 @@ int v4l2_async_register_subdev(struct v4 ret = v4l2_async_match_notify(notifier, v4l2_dev, sd, asd); if (ret) - goto err_unbind; + break; ret = v4l2_async_notifier_try_complete(notifier); if (ret) - goto err_unbind; + break; goto out_unlock; } @@ -572,26 +572,22 @@ int v4l2_async_register_subdev(struct v4 /* None matched, wait for hot-plugging */ list_add(&sd->async_list, &subdev_list); -out_unlock: - mutex_unlock(&list_lock); - - return 0; - -err_unbind: - /* - * Complete failed. Unbind the sub-devices bound through registering - * this async sub-device. - */ - subdev_notifier = v4l2_async_find_subdev_notifier(sd); - if (subdev_notifier) - v4l2_async_notifier_unbind_all_subdevs(subdev_notifier); - - if (sd->asd) - v4l2_async_notifier_call_unbind(notifier, sd, sd->asd); - v4l2_async_cleanup(sd); + if (ret) { + /* + * Complete failed. Unbind the sub-devices bound through registering + * this async sub-device. + */ + subdev_notifier = v4l2_async_find_subdev_notifier(sd); + if (subdev_notifier) + v4l2_async_notifier_unbind_all_subdevs(subdev_notifier); + + if (sd->asd) + v4l2_async_notifier_call_unbind(notifier, sd, sd->asd); + v4l2_async_cleanup(sd); + } +out_unlock: mutex_unlock(&list_lock); - return ret; } EXPORT_SYMBOL(v4l2_async_register_subdev);