2009-01-24 05:55:39

by Jaswinder Singh

[permalink] [raw]
Subject: Re: [PATCH 09/10] HID: add compat support

On Fri, May 16, 2008 at 3:19 PM, Jiri Slaby <[email protected]> wrote:
> Add compat option to hid code to allow loading of all modules on
> systems which don't allow autoloading because of old userspace.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> ---
> Documentation/feature-removal-schedule.txt | 7 +++++++
> drivers/hid/Kconfig | 11 +++++++++++
> drivers/hid/Makefile | 4 ++++
> drivers/hid/hid-apple.c | 2 ++
> drivers/hid/hid-core.c | 12 ++++++++++++
> drivers/hid/hid-dummy.c | 13 +++++++++++++
> drivers/hid/hid-logitech.c | 2 ++
> include/linux/hid.h | 17 +++++++++++++++--
> 8 files changed, 66 insertions(+), 2 deletions(-)
> create mode 100644 drivers/hid/hid-dummy.c
>

> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index c67e5fe..447aed9 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h

> +#ifdef CONFIG_HID_COMPAT
> +#define HID_COMPAT_LOAD_DRIVER(name) \
> +void hid_compat_##name(void) { } \
> +EXPORT_SYMBOL(hid_compat_##name)
> +#else
> +#define HID_COMPAT_LOAD_DRIVER(name)
> +#endif /* HID_COMPAT */
> +#define HID_COMPAT_CALL_DRIVER(name) do { \
> + extern void hid_compat_##name(void); \
> + hid_compat_##name(); \
> +} while (0)
> +
> #endif
>

This leads to 3 headers_check warnings:

usr/include/linux/hid.h:66: leaks CONFIG_HID to userspace where it is not valid
usr/include/linux/hid.h:69: extern's make no sense in userspace
usr/include/linux/hid.h:76: extern's make no sense in userspace

So I am moving whole above block in #ifdef __KEREL__

If you have any objections, then let me know, now it will look like this:

#endif /* HID_FF */

+#ifdef __KERNEL__
#ifdef CONFIG_HID_COMPAT
#define HID_COMPAT_LOAD_DRIVER(name) \
/* prototype to avoid sparse warning */ \
@@ -804,6 +805,7 @@ EXPORT_SYMBOL(hid_compat_##name)
extern void hid_compat_##name(void); \
hid_compat_##name(); \
} while (0)
+#endif /* __KERNEL__ */

#endif


--
JSR


2009-01-25 13:04:15

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH 09/10] HID: add compat support

Jaswinder Singh Rajput wrote:
> If you have any objections, then let me know, now it will look like this:

No objections from me, nice catch, thanks.

> #endif /* HID_FF */
>
> +#ifdef __KERNEL__
> #ifdef CONFIG_HID_COMPAT
> #define HID_COMPAT_LOAD_DRIVER(name) \
> /* prototype to avoid sparse warning */ \
> @@ -804,6 +805,7 @@ EXPORT_SYMBOL(hid_compat_##name)
> extern void hid_compat_##name(void); \
> hid_compat_##name(); \
> } while (0)
> +#endif /* __KERNEL__ */
>
> #endif

2009-01-25 16:57:22

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH 09/10] HID: add compat support

On Sat, 24 Jan 2009, Jaswinder Singh Rajput wrote:

> This leads to 3 headers_check warnings:
>
> usr/include/linux/hid.h:66: leaks CONFIG_HID to userspace where it is not valid
> usr/include/linux/hid.h:69: extern's make no sense in userspace
> usr/include/linux/hid.h:76: extern's make no sense in userspace
>
> So I am moving whole above block in #ifdef __KEREL__
>
> If you have any objections, then let me know, now it will look like this:
>
> #endif /* HID_FF */
>
> +#ifdef __KERNEL__
> #ifdef CONFIG_HID_COMPAT
> #define HID_COMPAT_LOAD_DRIVER(name) \
> /* prototype to avoid sparse warning */ \
> @@ -804,6 +805,7 @@ EXPORT_SYMBOL(hid_compat_##name)
> extern void hid_compat_##name(void); \
> hid_compat_##name(); \
> } while (0)
> +#endif /* __KERNEL__ */
>
> #endif

Thanks for catching this. Could you please resend the patch to me, with
proper changelog and Signed-off-by: line? Thanks.

--
Jiri Kosina
SUSE Labs

2009-01-25 17:12:33

by Jaswinder Singh Rajput

[permalink] [raw]
Subject: Re: [PATCH 09/10] HID: add compat support

On Sun, 2009-01-25 at 17:57 +0100, Jiri Kosina wrote:
> On Sat, 24 Jan 2009, Jaswinder Singh Rajput wrote:
>
> > This leads to 3 headers_check warnings:
> >
> > usr/include/linux/hid.h:66: leaks CONFIG_HID to userspace where it is not valid
> > usr/include/linux/hid.h:69: extern's make no sense in userspace
> > usr/include/linux/hid.h:76: extern's make no sense in userspace
> >
> > So I am moving whole above block in #ifdef __KEREL__
> >
> > If you have any objections, then let me know, now it will look like this:
> >
> > #endif /* HID_FF */
> >
> > +#ifdef __KERNEL__
> > #ifdef CONFIG_HID_COMPAT
> > #define HID_COMPAT_LOAD_DRIVER(name) \
> > /* prototype to avoid sparse warning */ \
> > @@ -804,6 +805,7 @@ EXPORT_SYMBOL(hid_compat_##name)
> > extern void hid_compat_##name(void); \
> > hid_compat_##name(); \
> > } while (0)
> > +#endif /* __KERNEL__ */
> >
> > #endif
>
> Thanks for catching this. Could you please resend the patch to me, with
> proper changelog and Signed-off-by: line? Thanks.
>

I have already send the patchset to Ingo "Re: [GIT PULL -tip] fix 22
make headers_check - 200901" and I also CCed to you [email protected]

--
JSR