2008-12-16 12:57:57

by Hannes Eder

[permalink] [raw]
Subject: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

Impact: include a prototype for the exported function in the macro

Fix about 20 of this warnings:

drivers/hid/hid-a4tech.c:162:1: warning: symbol 'hid_compat_a4tech' was not declared. Should it be static?

Signed-off-by: Hannes Eder <[email protected]>
---
include/linux/hid.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 215035b..81aa84d 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -793,6 +793,8 @@ dbg_hid(const char *fmt, ...)

#ifdef CONFIG_HID_COMPAT
#define HID_COMPAT_LOAD_DRIVER(name) \
+/* prototype to avoid sparse warning */ \
+extern void hid_compat_##name(void); \
void hid_compat_##name(void) { } \
EXPORT_SYMBOL(hid_compat_##name)
#else
--
1.5.6.3


2008-12-16 13:35:28

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On Tue, 16 Dec 2008, Hannes Eder wrote:

> Impact: include a prototype for the exported function in the macro
>
> Fix about 20 of this warnings:
>
> drivers/hid/hid-a4tech.c:162:1: warning: symbol 'hid_compat_a4tech' was not declared. Should it be static?
>
> Signed-off-by: Hannes Eder <[email protected]>

Applied, thanks.

--
Jiri Kosina
SUSE Labs

2008-12-16 15:58:50

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On Tue, Dec 16, 2008 at 01:31:31PM +0100, Hannes Eder wrote:
> Impact: include a prototype for the exported function in the macro
>
> Fix about 20 of this warnings:
>
> drivers/hid/hid-a4tech.c:162:1: warning: symbol 'hid_compat_a4tech' was not declared. Should it be static?
>
> Signed-off-by: Hannes Eder <[email protected]>
> ---
> include/linux/hid.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 215035b..81aa84d 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -793,6 +793,8 @@ dbg_hid(const char *fmt, ...)
>
> #ifdef CONFIG_HID_COMPAT
> #define HID_COMPAT_LOAD_DRIVER(name) \
> +/* prototype to avoid sparse warning */ \
> +extern void hid_compat_##name(void); \
> void hid_compat_##name(void) { } \

surely this could simply be changed to 'static void hid_compat_##name(void)'
if it is only being defined to be an empty function?

> EXPORT_SYMBOL(hid_compat_##name)
> #else
> --
> 1.5.6.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2008-12-16 16:01:51

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On Tue, 16 Dec 2008, Ben Dooks wrote:

> > #ifdef CONFIG_HID_COMPAT
> > #define HID_COMPAT_LOAD_DRIVER(name) \
> > +/* prototype to avoid sparse warning */ \
> > +extern void hid_compat_##name(void); \
> > void hid_compat_##name(void) { } \
> surely this could simply be changed to 'static void
> hid_compat_##name(void)' if it is only being defined to be an empty

This will cause gcc emit warnings about unused symbols.

--
Jiri Kosina
SUSE Labs

2008-12-16 19:41:40

by Ben Dooks

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On Tue, Dec 16, 2008 at 05:01:38PM +0100, Jiri Kosina wrote:
> On Tue, 16 Dec 2008, Ben Dooks wrote:
>
> > > #ifdef CONFIG_HID_COMPAT
> > > #define HID_COMPAT_LOAD_DRIVER(name) \
> > > +/* prototype to avoid sparse warning */ \
> > > +extern void hid_compat_##name(void); \
> > > void hid_compat_##name(void) { } \
> > surely this could simply be changed to 'static void
> > hid_compat_##name(void)' if it is only being defined to be an empty
>
> This will cause gcc emit warnings about unused symbols.

sorry, 'static inline void' then.

> --
> Jiri Kosina
> SUSE Labs
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/

--
Ben ([email protected], http://www.fluff.org/)

'a smiley only costs 4 bytes'

2008-12-16 19:45:24

by Hannes Eder

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On Tue, Dec 16, 2008 at 5:01 PM, Jiri Kosina <[email protected]> wrote:
> On Tue, 16 Dec 2008, Ben Dooks wrote:
>
>> > #ifdef CONFIG_HID_COMPAT
>> > #define HID_COMPAT_LOAD_DRIVER(name) \
>> > +/* prototype to avoid sparse warning */ \
>> > +extern void hid_compat_##name(void); \
>> > void hid_compat_##name(void) { } \
>> surely this could simply be changed to 'static void
>> hid_compat_##name(void)' if it is only being defined to be an empty
>
> This will cause gcc emit warnings about unused symbols.

The real problem is that the symbol (in that case the function) is not
exported if defined static.

e.g.:

$ cat >foobar.c <<EOF
> static void foo(void) {}
> extern void foo(void);
>
> void bar(void) {}
> EOF
$ gcc -c foobar.c
$ nm foobar.o
00000005 T bar
00000000 t foo

from the nm manpage:
The symbol type. ... If lowercase, the symbol is local; if
uppercase, the symbol is global (external)

i.e. foo is local

see http://lkml.org/lkml/2008/9/4/495 for what ANSI C Std has to say
about that and about the proposed change.

But, don't ask my why the void functions with empty function bodies
are exported.

On Tue, Dec 16, 2008 at 8:41 PM, Ben Dooks <[email protected]> wrote:
> sorry, 'static inline void' then.

in that case the symbol would not even be local as it is not used with
the translation unit.

$ nm
00000005 T bar

-Hannes

2008-12-19 08:31:21

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On 12/16/2008 08:44 PM, Hannes Eder wrote:
> But, don't ask my why the void functions with empty function bodies
> are exported.

Because of autoload to create a dependency. Note that this is a temporary
solution to workaround the non-presence of hid bus in module-init-tools and will
be removed in next few releases.

2008-12-19 09:37:44

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

On Fri, 19 Dec 2008, Jiri Slaby wrote:

> > But, don't ask my why the void functions with empty function bodies
> > are exported.
> Because of autoload to create a dependency. Note that this is a temporary
> solution to workaround the non-presence of hid bus in module-init-tools and will
> be removed in next few releases.

BTW while we are at it -- what is the status with module-init-tools? Has
the support been merged already?

Thanks,

--
Jiri Kosina
SUSE Labs

2008-12-20 16:56:24

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] HID: avoid sparse warning in HID_COMPAT_LOAD_DRIVER

Jiri Kosina wrote:
> BTW while we are at it -- what is the status with module-init-tools? Has
> the support been merged already?

As far as I can see, not yet. Jon, what's the status of hid bus support in
m-i-t, please?