Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

[Dan Carpenter]

Hello Guenter Roeck,

The patch 9aeb5c70e84d: "hwmon: (tmp401) Convert to use 
devm_hwmon_device_register_with_groups" from Sep 4, 2013, leads to 
the following warning:
"drivers/hwmon/tmp401.c:360 store_temp()
	 warn: 'data' isn't an ERR_PTR"

drivers/hwmon/tmp401.c
   349  static ssize_t store_temp(struct device *dev, struct device_attribute *devattr,
   350                            const char *buf, size_t count)
   351  {
   352          int nr = to_sensor_dev_attr_2(devattr)->nr;
   353          int index = to_sensor_dev_attr_2(devattr)->index;
   354          struct tmp401_data *data = dev_get_drvdata(dev);
                                           ^^^^^^^^^^^^^^^^^^^
Returns NULL.

   355          struct i2c_client *client = data->client;
                                            ^^^^^^^^^^^^
Dereference.

   356          long val;
   357          u16 reg;
   358          u8 regaddr;
   359  
   360          if (IS_ERR(data))
                    ^^^^^^^^^^^^
Can this test be removed?

   361                  return PTR_ERR(data);
   362  

regards,
dan carpenter


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

[Guenter Roeck]

On Fri, Oct 18, 2013 at 05:25:06PM +0300, Dan Carpenter wrote:
> Hello Guenter Roeck,
> 
> The patch 9aeb5c70e84d: "hwmon: (tmp401) Convert to use 
> devm_hwmon_device_register_with_groups" from Sep 4, 2013, leads to 
> the following warning:
> "drivers/hwmon/tmp401.c:360 store_temp()
> 	 warn: 'data' isn't an ERR_PTR"
> 
> drivers/hwmon/tmp401.c
>    349  static ssize_t store_temp(struct device *dev, struct device_attribute *devattr,
>    350                            const char *buf, size_t count)
>    351  {
>    352          int nr = to_sensor_dev_attr_2(devattr)->nr;
>    353          int index = to_sensor_dev_attr_2(devattr)->index;
>    354          struct tmp401_data *data = dev_get_drvdata(dev);
>                                            ^^^^^^^^^^^^^^^^^^^
> Returns NULL.
> 
>    355          struct i2c_client *client = data->client;
>                                             ^^^^^^^^^^^^
> Dereference.
> 
>    356          long val;
>    357          u16 reg;
>    358          u8 regaddr;
>    359  
>    360          if (IS_ERR(data))
>                     ^^^^^^^^^^^^
> Can this test be removed?
> 
Yes, it can. Leftover from earlier code.

Thanks for the note, I'll update the patch.

Did you see this with smatch ? I am running smatch on the branch myself,
but I don't recall seing the report, so I wonder.

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

[Dan Carpenter]

On Fri, Oct 18, 2013 at 09:13:53AM -0700, Guenter Roeck wrote:
> Did you see this with smatch ? I am running smatch on the branch myself,
> but I don't recall seing the report, so I wonder.

I have a check that I haven't pushed where it complains if we check
IS_ERR() on something that's not an error pointer.  It has a lot of
false positives.

I should write a check where it complains if we check IS_ERR() after a
dereference.

regards,
dan carpenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

[Dan Carpenter]

Hello Guenter Roeck,

The patch 42f309303902: "hwmon: (tmp401) Convert to use
devm_hwmon_device_register_with_groups" from Sep 4, 2013, leads to
the following
static checker warning: "drivers/hwmon/tmp401.c:360 store_temp()
	 warn: 'data' isn't an ERR_PTR"

drivers/hwmon/tmp401.c
   354          struct tmp401_data *data = dev_get_drvdata(dev);
   355          struct i2c_client *client = data->client;
   356          long val;
   357          u16 reg;
   358          u8 regaddr;
   359  
   360          if (IS_ERR(data))
   361                  return PTR_ERR(data);

dev_get_drvdata() returns NULL on error, but I'm not sure if that's
possible here.  Can the check be removed?

regards,
dan carpenter


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

[Guenter Roeck]

On Wed, Nov 06, 2013 at 10:28:41PM +0300, Dan Carpenter wrote:
> Hello Guenter Roeck,
> 
> The patch 42f309303902: "hwmon: (tmp401) Convert to use
> devm_hwmon_device_register_with_groups" from Sep 4, 2013, leads to
> the following
> static checker warning: "drivers/hwmon/tmp401.c:360 store_temp()
> 	 warn: 'data' isn't an ERR_PTR"
> 
> drivers/hwmon/tmp401.c
>    354          struct tmp401_data *data = dev_get_drvdata(dev);
>    355          struct i2c_client *client = data->client;
>    356          long val;
>    357          u16 reg;
>    358          u8 regaddr;
>    359  
>    360          if (IS_ERR(data))
>    361                  return PTR_ERR(data);
> 
> dev_get_drvdata() returns NULL on error, but I'm not sure if that's
> possible here.  Can the check be removed?
> 
Hi Dan,

I am a bit confused. I seem to recall that you reported this before,
and I am quite sure I had it fixed. Looking at my hwmon-next branch,
that code is no longer in there. linux-next doesn't have it either.
What repository are you checking ?

Thanks,
Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] hwmon: (tmp401) Convert to use devm_hwmon_device_register_with_groups

[Dan Carpenter]

On Wed, Nov 06, 2013 at 11:39:24AM -0800, Guenter Roeck wrote:
> On Wed, Nov 06, 2013 at 10:28:41PM +0300, Dan Carpenter wrote:
> > Hello Guenter Roeck,
> > 
> > The patch 42f309303902: "hwmon: (tmp401) Convert to use
> > devm_hwmon_device_register_with_groups" from Sep 4, 2013, leads to
> > the following
> > static checker warning: "drivers/hwmon/tmp401.c:360 store_temp()
> > 	 warn: 'data' isn't an ERR_PTR"
> > 
> > drivers/hwmon/tmp401.c
> >    354          struct tmp401_data *data = dev_get_drvdata(dev);
> >    355          struct i2c_client *client = data->client;
> >    356          long val;
> >    357          u16 reg;
> >    358          u8 regaddr;
> >    359  
> >    360          if (IS_ERR(data))
> >    361                  return PTR_ERR(data);
> > 
> > dev_get_drvdata() returns NULL on error, but I'm not sure if that's
> > possible here.  Can the check be removed?
> > 
> Hi Dan,
> 
> I am a bit confused. I seem to recall that you reported this before,
> and I am quite sure I had it fixed. Looking at my hwmon-next branch,
> that code is no longer in there. linux-next doesn't have it either.
> What repository are you checking ?

Oops.  Sorry about that.  I sent this by mistake.

regards,
dan carpenter


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors