diff mbox series

i2c: core: squelch error: uninitialized symbol 'ret'

Message ID 20181005161826.2455-1-george_davis@mentor.com
State Rejected
Headers show
Series i2c: core: squelch error: uninitialized symbol 'ret' | expand

Commit Message

George G. Davis Oct. 5, 2018, 4:18 p.m. UTC
The following smatch error was introduced by commit 7ca5f6be7900 ("i2c:
recovery: add get_bus_free callback"):

drivers/i2c/i2c-core-base.c:228 i2c_generic_scl_recovery() error: uninitialized symbol 'ret'.

Analysis of the code appears to show that the smatch error is a false
positive since 'ret' will in fact be initialized after exiting the
'while()' loop.

Squelch the smatch error since it is a false positive.

Fixes: 7ca5f6be7900 ("i2c: recovery: add get_bus_free callback")
Signed-off-by: George G. Davis <george_davis@mentor.com>
---
 drivers/i2c/i2c-core-base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Wolfram Sang Oct. 5, 2018, 4:28 p.m. UTC | #1
On Fri, Oct 05, 2018 at 12:18:25PM -0400, George G. Davis wrote:
> The following smatch error was introduced by commit 7ca5f6be7900 ("i2c:
> recovery: add get_bus_free callback"):
> 
> drivers/i2c/i2c-core-base.c:228 i2c_generic_scl_recovery() error: uninitialized symbol 'ret'.
> 
> Analysis of the code appears to show that the smatch error is a false
> positive since 'ret' will in fact be initialized after exiting the
> 'while()' loop.
> 
> Squelch the smatch error since it is a false positive.

What about fixing smatch?

> 
> Fixes: 7ca5f6be7900 ("i2c: recovery: add get_bus_free callback")
> Signed-off-by: George G. Davis <george_davis@mentor.com>
> ---
>  drivers/i2c/i2c-core-base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 9ee9a15e7134..7b25ecb6b616 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -185,7 +185,7 @@ static int i2c_generic_bus_free(struct i2c_adapter *adap)
>  int i2c_generic_scl_recovery(struct i2c_adapter *adap)
>  {
>  	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> -	int i = 0, scl = 1, ret;
> +	int i = 0, scl = 1, uninitialized_var(ret);
>  
>  	if (bri->prepare_recovery)
>  		bri->prepare_recovery(adap);
> -- 
> 2.14.4
>
George G. Davis Oct. 5, 2018, 4:45 p.m. UTC | #2
On Fri, Oct 05, 2018 at 06:28:41PM +0200, Wolfram Sang wrote:
> On Fri, Oct 05, 2018 at 12:18:25PM -0400, George G. Davis wrote:
> > The following smatch error was introduced by commit 7ca5f6be7900 ("i2c:
> > recovery: add get_bus_free callback"):
> > 
> > drivers/i2c/i2c-core-base.c:228 i2c_generic_scl_recovery() error: uninitialized symbol 'ret'.
> > 
> > Analysis of the code appears to show that the smatch error is a false
> > positive since 'ret' will in fact be initialized after exiting the
> > 'while()' loop.
> > 
> > Squelch the smatch error since it is a false positive.
> 
> What about fixing smatch?

That would perhaps be the better long term solution. Something tells me
that this topic comes up periodically. : ) For the record, I've
confirmed that this problem exists while using the latest version of
smatch [1] but I doubt my abilities to fix smatch. So I chose to squelch
it instead.

Feel free to ignore if this is a topic already discussed in other
threads. To be honest, I did not search the mailing list to determine if
this particular issue was already discussed.  


--
Regards,
George

[1] https://repo.or.cz/smatch.git/commit/79fe36620a7a3a45d1a51d62238da250fb8db920
> 
> > 
> > Fixes: 7ca5f6be7900 ("i2c: recovery: add get_bus_free callback")
> > Signed-off-by: George G. Davis <george_davis@mentor.com>
> > ---
> >  drivers/i2c/i2c-core-base.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 9ee9a15e7134..7b25ecb6b616 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -185,7 +185,7 @@ static int i2c_generic_bus_free(struct i2c_adapter *adap)
> >  int i2c_generic_scl_recovery(struct i2c_adapter *adap)
> >  {
> >  	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
> > -	int i = 0, scl = 1, ret;
> > +	int i = 0, scl = 1, uninitialized_var(ret);
> >  
> >  	if (bri->prepare_recovery)
> >  		bri->prepare_recovery(adap);
> > -- 
> > 2.14.4
> >
Wolfram Sang Oct. 5, 2018, 5:43 p.m. UTC | #3
> Feel free to ignore if this is a topic already discussed in other
> threads. To be honest, I did not search the mailing list to determine if
> this particular issue was already discussed.  

Not this particular issue, but in general we usually don't change
working code to silence false positives in code checkers.

But thanks for letting me know this is a false positive!

BTW next time CC Dan Carpenter, he might be interested to fix the issue
in smatch.
George G. Davis Oct. 5, 2018, 8:55 p.m. UTC | #4
On Fri, Oct 05, 2018 at 07:43:01PM +0200, Wolfram Sang wrote:
> 
> > Feel free to ignore if this is a topic already discussed in other
> > threads. To be honest, I did not search the mailing list to determine if
> > this particular issue was already discussed.  
> 
> Not this particular issue, but in general we usually don't change
> working code to silence false positives in code checkers.

Ok.

> But thanks for letting me know this is a false positive!

No worries. Thanks for having a look.

> BTW next time CC Dan Carpenter, he might be interested to fix the issue
> in smatch.

I'll try to remember this in the future.

Thanks again!

--
Regards,
George
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 9ee9a15e7134..7b25ecb6b616 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -185,7 +185,7 @@  static int i2c_generic_bus_free(struct i2c_adapter *adap)
 int i2c_generic_scl_recovery(struct i2c_adapter *adap)
 {
 	struct i2c_bus_recovery_info *bri = adap->bus_recovery_info;
-	int i = 0, scl = 1, ret;
+	int i = 0, scl = 1, uninitialized_var(ret);
 
 	if (bri->prepare_recovery)
 		bri->prepare_recovery(adap);