Re: [Cocci] [RFC] drop owner assignment from platform_drivers

2014-10-11 Thread Russell King - ARM Linux 
On Sat, Oct 11, 2014 at 06:56:51PM +0200, Wolfram Sang wrote:
 
   You got me wondering, though, that it could not be correct to call
   platform_driver_register() from the platform core instead of module
   init. I will check tomorrow. Still, this would be a bug independent of
   my series. Although I'd need to respin it if platform_driver_probe()
   needed a fix.
  
  Right, this seems to be a preexisting bug. platform_create_bundle 
  and platform_driver_probe will both overwrite the .owner field with
  NULL since they live in builtin code. They need to be replaced with
  __platform_driver_probe and __platform_driver_register that both
  take an extra owner argument passed down from the caller in the driver
  module.
 
 Yeah, that would be one solution. However, my personal favourite would
 meanwhile be to revert the commit that Russell mentioned. I think it is
 cleaner to have the owner explicitly set in the module rather than
 hidden away by a function call. However, grepping through include/linux,
 there are a few subsystems hiding it this way. So, it is a pattern
 somewhow. Oh well...

It really /ought/ to be consistent, because inconsistencies like that
will be a never-ending source of subtle mistakes.

Imagine what it would be like if the kernel was a complete mess of
functions with return type int where there was no predominant
pattern of returning negative errno numbers - where it was random
whether int-returning functions returned zero for failure, others
returned zero for success.  We would have to look up every single
function to check it's return style, and it would be a bigger problem
when reviewing code.

There is a lot of value for saving time and reducing errors to have a
consistent, simple and obvious methodology.

(That's not to say that it should be enforced draconian style - but
there'd better be a good reason to be different, rather than I think
it's better this way or my personal style is different.)

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] [RFC] drop owner assignment from platform_drivers

2014-10-10 Thread Russell King - ARM Linux 
On Fri, Oct 10, 2014 at 08:26:05PM +0200, Wolfram Sang wrote:
 platform_create_bundle() calls platform_driver_probe().
 platform_driver_probe() calls platform_driver_register().
 platform_driver_register() modifies driver.owner.
 
 So, it is correct from the point of view that it doesn't make sense to
 set the .owner field if it gets overwritten anyhow.
 
 You got me wondering, though, that it could not be correct to call
 platform_driver_register() from the platform core instead of module
 init. I will check tomorrow. Still, this would be a bug independent of
 my series. Although I'd need to respin it if platform_driver_probe()
 needed a fix.

This shows what the bad side-effect of people doing cleanups is.
This bug was introduced by:

commit 9447057eaff871dd7c63c808de761b8732407169
Author: Libo Chen clbchenlibo.c...@huawei.com
Date:   Sat May 25 12:40:50 2013 +0800

platform_device: use a macro instead of platform_driver_register

I found a lot of mistakes using struct platform_driver without owner
so I make a macro instead of the function platform_driver_register.
It can set owner in it, then guys don`t care about module owner again.

Signed-off-by: Libo Chen libo.c...@huawei.com
Signed-off-by: Greg Kroah-Hartman gre...@linuxfoundation.org

So, this patch subsituted one set of mistakes for another mistake...

-- 
FTTC broadband for 0.8mile line: currently at 9.5Mbps down 400kbps up
according to speedtest.net.
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci