re: Thermal: Update binding logic based on platform data

re: Thermal: Update binding logic based on platform data

[Dan Carpenter]

Hello Durgadoss R,

This is a semi-automatic email about new static checker warnings.

The patch 9b70dfa68ae8: "Thermal: Update binding logic based on 
platform data" from Sep 18, 2012, leads to the following Smatch 
complaint:

drivers/thermal/thermal_sys.c:292 bind_tz()
	 error: we previously assumed 'tzp' could be null (see line 283)

drivers/thermal/thermal_sys.c
   282		/* If there is no platform data, try to use ops->bind */
   283		if (!tzp && tz->ops->bind) {
                    ^^^^
New check.

   284			list_for_each_entry(pos, &thermal_cdev_list, node) {
   285				ret = tz->ops->bind(tz, pos);
   286				if (ret)
   287					print_bind_err_msg(tz, pos, ret);
   288			}
   289			goto exit;
   290		}
   291	
   292		if (!tzp->tbp)
                     ^^^^^^^^
New dereference.

   293			goto exit;
   294	

There are also some locking bugs which need to be fixed as well.

drivers/thermal/thermal_sys.c:268 bind_cdev() warn: inconsistent returns mutex:&thermal_list_lock: locked (256) unlocked (268)
drivers/thermal/thermal_sys.c:396 update_temperature() warn: inconsistent returns mutex:&tz->lock: locked (390) unlocked (396)

regards,
dan carpenter

RE: Thermal: Update binding logic based on platform data

[R, Durgadoss]

Hi,

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Wednesday, September 26, 2012 1:58 AM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-kernel@vger.kernel.org
> Subject: re: Thermal: Update binding logic based on platform data
> 
> Hello Durgadoss R,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 9b70dfa68ae8: "Thermal: Update binding logic based on
> platform data" from Sep 18, 2012, leads to the following Smatch
> complaint:

looking into this. Will submit appropriate fix patches soon.

Is there a way for me to check/verify these kind of warnings from
my side ? This way, I can do the check before I submit each patch.

Thanks,
Durga

> 
> drivers/thermal/thermal_sys.c:292 bind_tz()
> 	 error: we previously assumed 'tzp' could be null (see line 283)
> 
> drivers/thermal/thermal_sys.c
>    282		/* If there is no platform data, try to use ops->bind */
>    283		if (!tzp && tz->ops->bind) {
>                     ^^^^
> New check.
> 
>    284			list_for_each_entry(pos, &thermal_cdev_list, node)
> {
>    285				ret = tz->ops->bind(tz, pos);
>    286				if (ret)
>    287					print_bind_err_msg(tz, pos, ret);
>    288			}
>    289			goto exit;
>    290		}
>    291
>    292		if (!tzp->tbp)
>                      ^^^^^^^^
> New dereference.
> 
>    293			goto exit;
>    294
> 
> There are also some locking bugs which need to be fixed as well.
> 
> drivers/thermal/thermal_sys.c:268 bind_cdev() warn: inconsistent returns
> mutex:&thermal_list_lock: locked (256) unlocked (268)
> drivers/thermal/thermal_sys.c:396 update_temperature() warn:
> inconsistent returns mutex:&tz->lock: locked (390) unlocked (396)
> 
> regards,
> dan carpenter
> 

Re: Thermal: Update binding logic based on platform data

[Dan Carpenter]

On Wed, Sep 26, 2012 at 02:39:22PM +0000, R, Durgadoss wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > Sent: Wednesday, September 26, 2012 1:58 AM
> > To: R, Durgadoss
> > Cc: Zhang, Rui; linux-kernel@vger.kernel.org
> > Subject: re: Thermal: Update binding logic based on platform data
> > 
> > Hello Durgadoss R,
> > 
> > This is a semi-automatic email about new static checker warnings.
> > 
> > The patch 9b70dfa68ae8: "Thermal: Update binding logic based on
> > platform data" from Sep 18, 2012, leads to the following Smatch
> > complaint:
> 
> looking into this. Will submit appropriate fix patches soon.
> 
> Is there a way for me to check/verify these kind of warnings from
> my side ? This way, I can do the check before I submit each patch.
> 

Sure:

git clone git://repo.or.cz/smatch.git
cd smatch
make
(no make install needed)
cd /to/kernel/src/
~/path/to/smatch/smatch_scripts/kchecker drivers/thermal/thermal_sys.c

regards,
dan carpenter

RE: Thermal: Update binding logic based on platform data

[R, Durgadoss]

Hi,

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Wednesday, September 26, 2012 10:51 PM
> To: R, Durgadoss
> Cc: Zhang, Rui; linux-kernel@vger.kernel.org
> Subject: Re: Thermal: Update binding logic based on platform data
> 
> On Wed, Sep 26, 2012 at 02:39:22PM +0000, R, Durgadoss wrote:
> > Hi,
> >
> > > -----Original Message-----
> > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> > > Sent: Wednesday, September 26, 2012 1:58 AM
> > > To: R, Durgadoss
> > > Cc: Zhang, Rui; linux-kernel@vger.kernel.org
> > > Subject: re: Thermal: Update binding logic based on platform data
> > >
> > > Hello Durgadoss R,
> > >
> > > This is a semi-automatic email about new static checker warnings.
> > >
> > > The patch 9b70dfa68ae8: "Thermal: Update binding logic based on
> > > platform data" from Sep 18, 2012, leads to the following Smatch
> > > complaint:
> >
> > looking into this. Will submit appropriate fix patches soon.
> >
> > Is there a way for me to check/verify these kind of warnings from
> > my side ? This way, I can do the check before I submit each patch.
> >
> 
> Sure:
> 
> git clone git://repo.or.cz/smatch.git
> cd smatch
> make
> (no make install needed)
> cd /to/kernel/src/
> ~/path/to/smatch/smatch_scripts/kchecker drivers/thermal/thermal_sys.c

Thanks Dan, That worked great, and I just submitted a patch fixing all the
warnings from smatch.

I will ensure that I run this before I submit patches, going forward.
Thanks for a nice tool :-)

Thanks,
Durga