Re: [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning
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
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
Reviewed-by: Niklas Söderlund
Signed-off-by: Sakari Ailus
Signed-off-by: Mauro Carvalho Chehab
--
Sakari Ailus
e-mail: sakari.ai...@iki.fi
Re: [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning
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. -- Regards, Laurent Pinchart
Re: [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning
Em Thu, 02 Nov 2017 04:51:40 +0200
Laurent Pinchart escreveu:
> Hi Mauro,
>
> Thank you for the patch.
>
> 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.
Thanks,
Mauro
media: v4l2-async: shut up an unitialized symbol warning
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. Yet, the logic is more
complex than it should. So, rework it to make clearer about
what happens when ret != 0.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/v4l2-async.c | 38 +++
1 file changed, 17 insertions(+), 21 deletions(-)
--- 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);
Re: [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning
On Thu, Nov 02, 2017 at 04:51:40AM +0200, Laurent Pinchart wrote: > Hi Mauro, > > Thank you for the patch. > > 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 another ret defined in a block under this one; removing that is the correct fix. I wonder why GCC didn't complain about that to begin with... usually it does. -- Sakari Ailus e-mail: sakari.ai...@iki.fi
Re: [PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning
Hi Mauro,
Thank you for the patch.
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.
> Signed-off-by: Mauro Carvalho Chehab
> ---
> drivers/media/v4l2-core/v4l2-async.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-async.c
> b/drivers/media/v4l2-core/v4l2-async.c index 49f7eccc76db..5bc861c4ae5c
> 100644
> --- a/drivers/media/v4l2-core/v4l2-async.c
> +++ b/drivers/media/v4l2-core/v4l2-async.c
> @@ -532,7 +532,7 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
> {
> struct v4l2_async_notifier *subdev_notifier;
> struct v4l2_async_notifier *notifier;
> - int ret;
> + int uninitialized_var(ret);
>
> /*
>* No reference taken. The reference is held by the device
--
Regards,
Laurent Pinchart
[PATCH v2 08/26] media: v4l2-async: shut up an unitialized symbol warning
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.
Signed-off-by: Mauro Carvalho Chehab
---
drivers/media/v4l2-core/v4l2-async.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-async.c
b/drivers/media/v4l2-core/v4l2-async.c
index 49f7eccc76db..5bc861c4ae5c 100644
--- a/drivers/media/v4l2-core/v4l2-async.c
+++ b/drivers/media/v4l2-core/v4l2-async.c
@@ -532,7 +532,7 @@ int v4l2_async_register_subdev(struct v4l2_subdev *sd)
{
struct v4l2_async_notifier *subdev_notifier;
struct v4l2_async_notifier *notifier;
- int ret;
+ int uninitialized_var(ret);
/*
* No reference taken. The reference is held by the device
--
2.13.6
