re: power_supply: Change ownership from driver to core

re: power_supply: Change ownership from driver to core

[Dan Carpenter]

Hello Krzysztof Kozlowski,

The patch 297d716f6260: "power_supply: Change ownership from driver
to core" from Mar 12, 2015, leads to the following static checker
warning:

	drivers/hid/hid-input.c:457 hidinput_setup_battery()
	error: potential NULL dereference 'dev->battery'.

drivers/hid/hid-input.c
   448          dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
   449          if (IS_ERR(dev->battery)) {
   450                  hid_warn(dev, "can't register power supply: %ld\n",
   451                                  PTR_ERR(dev->battery));
   452                  kfree(psy_desc->name);
   453                  kfree(psy_desc);
   454                  dev->battery = NULL;

Set to NULL.

   455          }
   456  
   457          power_supply_powers(dev->battery, &dev->dev);

Dereferenced inside function call.

   458  
   459  out:
   460          return true;
   461  }

regards,
dan carpenter

Re: power_supply: Change ownership from driver to core

[Krzysztof Kozlowski]

On pon, 2015-03-23 at 17:54 +0300, Dan Carpenter wrote:
> Hello Krzysztof Kozlowski,
> 
> The patch 297d716f6260: "power_supply: Change ownership from driver
> to core" from Mar 12, 2015, leads to the following static checker
> warning:
> 
> 	drivers/hid/hid-input.c:457 hidinput_setup_battery()
> 	error: potential NULL dereference 'dev->battery'.
> 

Yes, indeed. I'll fix it.

I suppose Smatch detected this so I wonder why it does not print this
warning in my configuration (I tried v0.5.0-2916-gbdbeaee0221e and
v0.5.0-3008-g08de8666209d). Any ideas?

Best regards,
Krzysztof

Re: power_supply: Change ownership from driver to core

[Dan Carpenter]

On Mon, Mar 23, 2015 at 04:21:48PM +0100, Krzysztof Kozlowski wrote:
> On pon, 2015-03-23 at 17:54 +0300, Dan Carpenter wrote:
> > Hello Krzysztof Kozlowski,
> > 
> > The patch 297d716f6260: "power_supply: Change ownership from driver
> > to core" from Mar 12, 2015, leads to the following static checker
> > warning:
> > 
> > 	drivers/hid/hid-input.c:457 hidinput_setup_battery()
> > 	error: potential NULL dereference 'dev->battery'.
> > 
> 
> Yes, indeed. I'll fix it.
> 
> I suppose Smatch detected this so I wonder why it does not print this
> warning in my configuration (I tried v0.5.0-2916-gbdbeaee0221e and
> v0.5.0-3008-g08de8666209d

That check in the released code buggy is buggy so it doesn't print many
warnings.  When I fixed it, then it prints warnings which look like
real bugs but are false positives.  I'm debating if I should just delete
the check.

Also I was just about to email about this one.

        drivers/power/charger-manager.c:1775 charger_manager_probe()
        error: 'cm->charger_psy' dereferencing possible ERR_PTR()

drivers/power/charger-manager.c
  1770  
  1771          cm->charger_psy = power_supply_register(NULL, &cm->charger_psy_desc,
  1772                                                  &psy_cfg);
  1773          if (IS_ERR(cm->charger_psy)) {
  1774                  dev_err(&pdev->dev, "Cannot register charger-manager with name \"%s\"\n",
  1775                          cm->charger_psy->desc->name);
                                ^^^^^^^^^^^^^^^
  1776                  return PTR_ERR(cm->charger_psy);
  1777          }
  1778  

regards,
dan carpenter