[PATCH] drivers/base - fix sparse warnings

[PATCH] drivers/base - fix sparse warnings

[Ben Dooks]

[-- Attachment #1: Type: text/plain, Size: 2502 bytes --]

There are a number of sparse warnings from the latest sparse
snapshot being generated from the drivers/base build. The
main culprits are due to the initialisation functions not
being declared in a header file. 

Also, the firmware.c file should include <linux/device.h>
to get the prototype of  firmware_register() and 
firmware_unregister().

This patch moves the init function declerations from the
init.c file to the base.h, and ensures it is included in
all the relevant c sources. It also adds <linux/device.h>
to the included headers for firmware.c. 

The patch does not solve all the sparse errors generated,
but reduces the count significantly.

drivers/base/core.c:161:1: warning: symbol 'devices_subsys' was not declared. Should it be static?
drivers/base/core.c:417:12: warning: symbol 'devices_init' was not declared. Should it be static?
drivers/base/sys.c:253:6: warning: symbol 'sysdev_shutdown' was not declared. Should it be static?
drivers/base/sys.c:326:5: warning: symbol 'sysdev_suspend' was not declared. Should it be static?
drivers/base/sys.c:428:5: warning: symbol 'sysdev_resume' was not declared. Should it be static?
drivers/base/sys.c:450:12: warning: symbol 'system_bus_init' was not declared. Should it be static?
drivers/base/bus.c:133:1: warning: symbol 'bus_subsys' was not declared. Should it be static?
drivers/base/bus.c:667:12: warning: symbol 'buses_init' was not declared. Should it be static?
drivers/base/class.c:759:12: warning: symbol 'classes_init' was not declared. Should it be static?
drivers/base/platform.c:313:12: warning: symbol 'platform_bus_init' was not declared. Should it be static?
drivers/base/cpu.c:110:12: warning: symbol 'cpu_dev_init' was not declared. Should it be static?
drivers/base/firmware.c:17:5: warning: symbol 'firmware_register' was not declared. Should it be static?
drivers/base/firmware.c:23:6: warning: symbol 'firmware_unregister' was not declared. Should it be static?
drivers/base/firmware.c:28:12: warning: symbol 'firmware_init' was not declared. Should it be static?
drivers/base/init.c:28:13: warning: symbol 'driver_init' was not declared. Should it be static?
drivers/base/dmapool.c:174:10: warning: implicit cast from nocast type
drivers/base/attribute_container.c:439:1: warning: symbol 'attribute_container_init' was not declared. Should it be static?
drivers/base/power/runtime.c:76:6: warning: symbol 'dpm_set_power_state' was not declared. Should it be static?

Signed-off-by: Ben Dooks <ben-linux@fluff.org>

[-- Attachment #2: 2614-base-sparse-fixes.patch --]
[-- Type: text/plain, Size: 3347 bytes --]

diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/attribute_container.c linux-2.6.14-rc4-bjd2/drivers/base/attribute_container.c
--- linux-2.6.14-rc4-bjd1/drivers/base/attribute_container.c	2005-10-11 10:56:31.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/attribute_container.c	2005-10-13 15:30:01.000000000 +0100
@@ -19,6 +19,8 @@
 #include <linux/list.h>
 #include <linux/module.h>
 
+#include "base.h"
+
 /* This is a private structure used to tie the classdev and the
  * container .. it should never be visible outside this file */
 struct internal_container {
diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/base.h linux-2.6.14-rc4-bjd2/drivers/base/base.h
--- linux-2.6.14-rc4-bjd1/drivers/base/base.h	2005-09-01 21:02:36.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/base.h	2005-10-13 15:26:56.000000000 +0100
@@ -1,3 +1,15 @@
+
+/* initialisation functions */
+
+extern int devices_init(void);
+extern int buses_init(void);
+extern int classes_init(void);
+extern int firmware_init(void);
+extern int platform_bus_init(void);
+extern int system_bus_init(void);
+extern int cpu_dev_init(void);
+extern int attribute_container_init(void);
+
 extern int bus_add_device(struct device * dev);
 extern void bus_remove_device(struct device * dev);
 
diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/cpu.c linux-2.6.14-rc4-bjd2/drivers/base/cpu.c
--- linux-2.6.14-rc4-bjd1/drivers/base/cpu.c	2005-09-01 21:02:36.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/cpu.c	2005-10-13 15:28:58.000000000 +0100
@@ -9,6 +9,7 @@
 #include <linux/topology.h>
 #include <linux/device.h>
 
+#include "base.h"
 
 struct sysdev_class cpu_sysdev_class = {
 	set_kset_name("cpu"),
diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/firmware.c linux-2.6.14-rc4-bjd2/drivers/base/firmware.c
--- linux-2.6.14-rc4-bjd1/drivers/base/firmware.c	2005-06-17 20:48:29.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/firmware.c	2005-10-13 15:28:08.000000000 +0100
@@ -11,6 +11,9 @@
 #include <linux/kobject.h>
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/device.h>
+
+#include "base.h"
 
 static decl_subsys(firmware, NULL, NULL);
 
diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/init.c linux-2.6.14-rc4-bjd2/drivers/base/init.c
--- linux-2.6.14-rc4-bjd1/drivers/base/init.c	2005-06-17 20:48:29.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/init.c	2005-10-13 15:27:05.000000000 +0100
@@ -10,14 +10,8 @@
 #include <linux/device.h>
 #include <linux/init.h>
 
-extern int devices_init(void);
-extern int buses_init(void);
-extern int classes_init(void);
-extern int firmware_init(void);
-extern int platform_bus_init(void);
-extern int system_bus_init(void);
-extern int cpu_dev_init(void);
-extern int attribute_container_init(void);
+#include "base.h"
+
 /**
  *	driver_init - initialize driver model.
  *
diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/platform.c linux-2.6.14-rc4-bjd2/drivers/base/platform.c
--- linux-2.6.14-rc4-bjd1/drivers/base/platform.c	2005-10-11 10:56:31.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/platform.c	2005-10-13 15:28:31.000000000 +0100
@@ -17,6 +17,8 @@
 #include <linux/bootmem.h>
 #include <linux/err.h>
 
+#include "base.h"
+
 struct device platform_bus = {
 	.bus_id		= "platform",
 };

Re: [PATCH] drivers/base - fix sparse warnings

[Linus Torvalds]

On Thu, 13 Oct 2005, Ben Dooks wrote:
> 
> The patch does not solve all the sparse errors generated,
> but reduces the count significantly.

Well, you should also then remove the _bad_ declarations.

For example, attribute_container_init() right now is defined in 
attribute_container.c, but then it's _declared_ (with no checking) where 
it's used in init.c. 

The sparse warnign is appropriate: it was not declared where that 
declaration is actually visible to the definition, so the code basically 
isn't type-safe at all (since there's nothing that enforces the 
declaration actually matching the definition).

You made the declaration properly visible, but you should also remove the 
bogus declaration. A declaration that isn't visible to the definition is 
always bad - since in the absense of a compiler with global visibility it 
may or may not actually match what it supposedly declares.

I wonder if I should make sparse warn about multiple declarations..

These days, sparse actually has some limited support for checking _global_ 
visibility, and we could do cross-checking across thousands of files. 
However, the build environment isn't really very amenable to that, so 
doing a global sparse check is pretty hard in practice.

We could possibly do a per-directory global check, which might be better 
than nothing (ie if you were to have incorrect declaration in a C file 
that is in the same directory as another C file, then we could 
cross-check).

But the kernel kbuild environment is pretty hairy, and I wouldn't even 
know where to begin trying to do that. It's also fundamentally hard to do 
if there are per-file pre-defines (since to do a cross-check, sparse wants 
to see all C files together on the command line).

		Linus

Re: [PATCH] drivers/base - fix sparse warnings

[Russell King]

On Thu, Oct 13, 2005 at 11:10:15AM -0700, Linus Torvalds wrote:
> On Thu, 13 Oct 2005, Ben Dooks wrote:
> > 
> > The patch does not solve all the sparse errors generated,
> > but reduces the count significantly.
> 
> Well, you should also then remove the _bad_ declarations.
>
> ...
> 
> You made the declaration properly visible, but you should also remove the 
> bogus declaration. A declaration that isn't visible to the definition is 
> always bad - since in the absense of a compiler with global visibility it 
> may or may not actually match what it supposedly declares.

Erm, lets take your example - attribute_container_init().  It's defined
in attribute_container.c, where the base.h include was added:

diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/attribute_container.c linux-2.6.14-rc4-bjd2/drivers/base/attribute_container.c
--- linux-2.6.14-rc4-bjd1/drivers/base/attribute_container.c	2005-10-11 10:56:31.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/attribute_container.c	2005-10-13 15:30:01.000000000 +0100
@@ -19,6 +19,8 @@
 #include <linux/list.h>
 #include <linux/module.h>
 
+#include "base.h"
+
 /* This is a private structure used to tie the classdev and the
  * container .. it should never be visible outside this file */
 struct internal_container {

The base.h include contains the definition:

diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/base.h linux-2.6.14-rc4-bjd2/drivers/base/base.h
--- linux-2.6.14-rc4-bjd1/drivers/base/base.h	2005-09-01 21:02:36.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/base.h	2005-10-13 15:26:56.000000000 +0100
@@ -1,3 +1,15 @@
+
+/* initialisation functions */
+
+extern int devices_init(void);
+extern int buses_init(void);
+extern int classes_init(void);
+extern int firmware_init(void);
+extern int platform_bus_init(void);
+extern int system_bus_init(void);
+extern int cpu_dev_init(void);
+extern int attribute_container_init(void);
+
 extern int bus_add_device(struct device * dev);
 extern void bus_remove_device(struct device * dev);
 
And base.h was included in init.c and the bogus declaration removed:

diff -urpN -X ../dontdiff linux-2.6.14-rc4-bjd1/drivers/base/init.c linux-2.6.14-rc4-bjd2/drivers/base/init.c
--- linux-2.6.14-rc4-bjd1/drivers/base/init.c	2005-06-17 20:48:29.000000000 +0100
+++ linux-2.6.14-rc4-bjd2/drivers/base/init.c	2005-10-13 15:27:05.000000000 +0100
@@ -10,14 +10,8 @@
 #include <linux/device.h>
 #include <linux/init.h>
 
-extern int devices_init(void);
-extern int buses_init(void);
-extern int classes_init(void);
-extern int firmware_init(void);
-extern int platform_bus_init(void);
-extern int system_bus_init(void);
-extern int cpu_dev_init(void);
-extern int attribute_container_init(void);
+#include "base.h"
+
 /**
  *	driver_init - initialize driver model.
  *

I can't see anything that was missed.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:  2.6 Serial core

Re: [PATCH] drivers/base - fix sparse warnings

[Ben Dooks]

On Thu, Oct 13, 2005 at 11:10:15AM -0700, Linus Torvalds wrote:
> 
> 
> On Thu, 13 Oct 2005, Ben Dooks wrote:
> > 
> > The patch does not solve all the sparse errors generated,
> > but reduces the count significantly.
> 
> Well, you should also then remove the _bad_ declarations.

Sorry, I do not follow you, can you clarify this for me.
My patch did not generate any more errors, just removed the ones
that where easy to see a solution too.
 
> For example, attribute_container_init() right now is defined in 
> attribute_container.c, but then it's _declared_ (with no checking) where 
> it's used in init.c. 
> 
> The sparse warnign is appropriate: it was not declared where that 
> declaration is actually visible to the definition, so the code basically 
> isn't type-safe at all (since there's nothing that enforces the 
> declaration actually matching the definition).
> 
> You made the declaration properly visible, but you should also remove the 
> bogus declaration. A declaration that isn't visible to the definition is 
> always bad - since in the absense of a compiler with global visibility it 
> may or may not actually match what it supposedly declares.
> 
> I wonder if I should make sparse warn about multiple declarations..

I pulled the old declerations out of drivers/base/init.c ? I'm sure
the patch shows that?

-- 
Ben (ben@fluff.org, http://www.fluff.org/)

  'a smiley only costs 4 bytes'

Re: [PATCH] drivers/base - fix sparse warnings

[Linus Torvalds]

On Thu, 13 Oct 2005, Russell King wrote:
> 
> Erm, lets take your example - attribute_container_init().  It's defined
> in attribute_container.c, where the base.h include was added:
> 
> I can't see anything that was missed.

Duh. My mistake, I looked at the patch twice, but I still missed the place 
where it removed the declaration. Twice.

Gaah. Where are my brain-pills again?

		Linus

[PATCH] drivers/base - fix sparse warnings

[Greg KH]

[PATCH] drivers/base - fix sparse warnings

There are a number of sparse warnings from the latest sparse
snapshot being generated from the drivers/base build. The
main culprits are due to the initialisation functions not
being declared in a header file.

Also, the firmware.c file should include <linux/device.h>
to get the prototype of  firmware_register() and
firmware_unregister().

This patch moves the init function declerations from the
init.c file to the base.h, and ensures it is included in
all the relevant c sources. It also adds <linux/device.h>
to the included headers for firmware.c.

The patch does not solve all the sparse errors generated,
but reduces the count significantly.

drivers/base/core.c:161:1: warning: symbol 'devices_subsys' was not declared. Should it be static?
drivers/base/core.c:417:12: warning: symbol 'devices_init' was not declared. Should it be static?
drivers/base/sys.c:253:6: warning: symbol 'sysdev_shutdown' was not declared. Should it be static?
drivers/base/sys.c:326:5: warning: symbol 'sysdev_suspend' was not declared. Should it be static?
drivers/base/sys.c:428:5: warning: symbol 'sysdev_resume' was not declared. Should it be static?
drivers/base/sys.c:450:12: warning: symbol 'system_bus_init' was not declared. Should it be static?
drivers/base/bus.c:133:1: warning: symbol 'bus_subsys' was not declared. Should it be static?
drivers/base/bus.c:667:12: warning: symbol 'buses_init' was not declared. Should it be static?
drivers/base/class.c:759:12: warning: symbol 'classes_init' was not declared. Should it be static?
drivers/base/platform.c:313:12: warning: symbol 'platform_bus_init' was not declared. Should it be static?
drivers/base/cpu.c:110:12: warning: symbol 'cpu_dev_init' was not declared. Should it be static?
drivers/base/firmware.c:17:5: warning: symbol 'firmware_register' was not declared. Should it be static?
drivers/base/firmware.c:23:6: warning: symbol 'firmware_unregister' was not declared. Should it be static?
drivers/base/firmware.c:28:12: warning: symbol 'firmware_init' was not declared. Should it be static?
drivers/base/init.c:28:13: warning: symbol 'driver_init' was not declared. Should it be static?
drivers/base/dmapool.c:174:10: warning: implicit cast from nocast type
drivers/base/attribute_container.c:439:1: warning: symbol 'attribute_container_init' was not declared. Should it be static?
drivers/base/power/runtime.c:76:6: warning: symbol 'dpm_set_power_state' was not declared. Should it be static?

Signed-off-by: Ben Dooks <ben-linux@fluff.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

---
commit e9821c685cbf2d72f6d692117e83ff9b71c3315b
tree dd96f48d3337746850b8c382aa71410036a3d519
parent 104d94a7ef01bb62f2be1d59c7580c1a6f3478d5
author Ben Dooks <ben-linux@fluff.org> Thu, 13 Oct 2005 17:54:41 +0100
committer Greg Kroah-Hartman <gregkh@suse.de> Thu, 27 Oct 2005 22:48:07 -0700

 drivers/base/attribute_container.c |    2 ++
 drivers/base/base.h                |   12 ++++++++++++
 drivers/base/cpu.c                 |    1 +
 drivers/base/firmware.c            |    3 +++
 drivers/base/init.c                |   10 ++--------
 drivers/base/platform.c            |    2 ++
 6 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/base/attribute_container.c b/drivers/base/attribute_container.c
index 6b2eb6f..2a7d7ae 100644
--- a/drivers/base/attribute_container.c
+++ b/drivers/base/attribute_container.c
@@ -19,6 +19,8 @@
 #include <linux/list.h>
 #include <linux/module.h>
 
+#include "base.h"
+
 /* This is a private structure used to tie the classdev and the
  * container .. it should never be visible outside this file */
 struct internal_container {
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 783752b..e3b548d 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -1,3 +1,15 @@
+
+/* initialisation functions */
+
+extern int devices_init(void);
+extern int buses_init(void);
+extern int classes_init(void);
+extern int firmware_init(void);
+extern int platform_bus_init(void);
+extern int system_bus_init(void);
+extern int cpu_dev_init(void);
+extern int attribute_container_init(void);
+
 extern int bus_add_device(struct device * dev);
 extern void bus_remove_device(struct device * dev);
 
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index b79badd..081c927 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -9,6 +9,7 @@
 #include <linux/topology.h>
 #include <linux/device.h>
 
+#include "base.h"
 
 struct sysdev_class cpu_sysdev_class = {
 	set_kset_name("cpu"),
diff --git a/drivers/base/firmware.c b/drivers/base/firmware.c
index 88ab044..cb1b98a 100644
--- a/drivers/base/firmware.c
+++ b/drivers/base/firmware.c
@@ -11,6 +11,9 @@
 #include <linux/kobject.h>
 #include <linux/module.h>
 #include <linux/init.h>
+#include <linux/device.h>
+
+#include "base.h"
 
 static decl_subsys(firmware, NULL, NULL);
 
diff --git a/drivers/base/init.c b/drivers/base/init.c
index a76ae5a..84e604e 100644
--- a/drivers/base/init.c
+++ b/drivers/base/init.c
@@ -10,14 +10,8 @@
 #include <linux/device.h>
 #include <linux/init.h>
 
-extern int devices_init(void);
-extern int buses_init(void);
-extern int classes_init(void);
-extern int firmware_init(void);
-extern int platform_bus_init(void);
-extern int system_bus_init(void);
-extern int cpu_dev_init(void);
-extern int attribute_container_init(void);
+#include "base.h"
+
 /**
  *	driver_init - initialize driver model.
  *
diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index 361e204..a1a56ff 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -17,6 +17,8 @@
 #include <linux/bootmem.h>
 #include <linux/err.h>
 
+#include "base.h"
+
 struct device platform_bus = {
 	.bus_id		= "platform",
 };