[PATCH] sound/oss/sb_common.c: fix casting warning

[PATCH] sound/oss/sb_common.c: fix casting warning

[Jeff Garzik]

sound/oss/sb_common.c: In function 'probe_sbmpu':
sound/oss/sb_common.c:1231: warning: cast to pointer from integer of different size

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 sound/oss/sb_common.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/sound/oss/sb_common.c b/sound/oss/sb_common.c
index 07cbacf..77d0e5e 100644
--- a/sound/oss/sb_common.c
+++ b/sound/oss/sb_common.c
@@ -1228,7 +1228,8 @@ int probe_sbmpu(struct address_info *hw_config, struct module *owner)
 		}
 		attach_mpu401(hw_config, owner);
 		if (last_sb->irq == -hw_config->irq)
-			last_sb->midi_irq_cookie=(void *)hw_config->slots[1];
+			last_sb->midi_irq_cookie =
+				(void *)(long) hw_config->slots[1];
 		return 1;
 	}
 #endif
-- 
1.5.2.4

[PATCH] Permit silencing of __deprecated warnings.

[Jeff Garzik]

The __deprecated marker is quite useful in highlighting the remnants of
old APIs that want removing.

However, it is quite normal for one or more years to pass, before the
(usually ancient, bitrotten) code in question is either updated or
deleted.

Thus, like __must_check, add a Kconfig option that permits the silencing
of this compiler warning.

This change mimics the ifdef-ery and Kconfig defaults of MUST_CHECK as
closely as possible.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 include/linux/compiler.h |    6 ++++++
 lib/Kconfig.debug        |    8 ++++++++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index c811c8b..c68b67b 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -101,6 +101,12 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 #undef __must_check
 #define __must_check
 #endif
+#ifndef CONFIG_ENABLE_WARN_DEPRECATED
+#undef __deprecated
+#undef __deprecated_for_modules
+#define __deprecated
+#define __deprecated_for_modules
+#endif
 
 /*
  * Allow us to avoid 'defined but not used' warnings on functions and data,
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 1faa508..1e5f207 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -9,6 +9,14 @@ config PRINTK_TIME
 	  operations.  This is useful for identifying long delays
 	  in kernel startup.
 
+config ENABLE_WARN_DEPRECATED
+	bool "Enable __deprecated logic"
+	default y
+	help
+	  Enable the __deprecated logic in the kernel build.
+	  Disable this to suppress the "warning: 'foo' is deprecated
+	  (declared at kernel/power/somefile.c:1234)" messages.
+
 config ENABLE_MUST_CHECK
 	bool "Enable __must_check logic"
 	default y
-- 
1.5.2.4

Re: [PATCH] Permit silencing of __deprecated warnings.

[Andrew Morton]

On Thu, 25 Oct 2007 04:06:13 -0400 (EDT) Jeff Garzik <jeff@garzik.org> wrote:

> The __deprecated marker is quite useful in highlighting the remnants of
> old APIs that want removing.
> 
> However, it is quite normal for one or more years to pass, before the
> (usually ancient, bitrotten) code in question is either updated or
> deleted.
> 
> Thus, like __must_check, add a Kconfig option that permits the silencing
> of this compiler warning.
> 
> This change mimics the ifdef-ery and Kconfig defaults of MUST_CHECK as
> closely as possible.

Sigh.  Can't we just fix the dud code?  Or mark it BROKEN and see what
happens?

Re: [PATCH] Permit silencing of __deprecated warnings.

[Jeff Garzik]

Andrew Morton wrote:
> On Thu, 25 Oct 2007 04:06:13 -0400 (EDT) Jeff Garzik <jeff@garzik.org> wrote:
> 
>> The __deprecated marker is quite useful in highlighting the remnants of
>> old APIs that want removing.
>>
>> However, it is quite normal for one or more years to pass, before the
>> (usually ancient, bitrotten) code in question is either updated or
>> deleted.
>>
>> Thus, like __must_check, add a Kconfig option that permits the silencing
>> of this compiler warning.
>>
>> This change mimics the ifdef-ery and Kconfig defaults of MUST_CHECK as
>> closely as possible.
> 
> Sigh.  Can't we just fix the dud code?  Or mark it BROKEN and see what
> happens?

__deprecated has spread to just about every API that people don't 
consider fresh and up-to-date.

Like I noted in the patch description, rewriting grotty ISA/MCA/etc. 
probe code is a thankless, boring task that few are crazy enough to 
attempt :)

As you can see from the patch flood recently I /have/ been working 
through the dud code, but it will still take years.  The changes 
required for each are on average ~200 LOC changed, if not more.

But regardless...  I don't see any reason to force every kernel build to 
remind us of grotty drivers.  Where's the benefit?  Everybody knows they 
are grotty.

Like __must_check this option defaults to the current state of things -- 
warnings -- so you have to take an extra step to turn them off.

	Jeff

Re: [PATCH] Permit silencing of __deprecated warnings.

[Linus Torvalds]

On Thu, 25 Oct 2007, Andrew Morton wrote:
> 
> Sigh.  Can't we just fix the dud code?  Or mark it BROKEN and see what
> happens?

I think removing __deprecated is the better option.

Quite frankly, some people add "__deprecated" *way* too eagerly. 

			Linus

Re: [PATCH] Permit silencing of __deprecated warnings.

[Sam Ravnborg]

On Thu, Oct 25, 2007 at 04:06:13AM -0400, Jeff Garzik wrote:
> The __deprecated marker is quite useful in highlighting the remnants of
> old APIs that want removing.
> 
> However, it is quite normal for one or more years to pass, before the
> (usually ancient, bitrotten) code in question is either updated or
> deleted.

Reminded me of a patch I have had floating around for far too long time.

Snippet from the mail:
--------------------------
From: Arnd Bergmann <arnd@arndb.de>

On Sünndag 07 August 2005 20:26, Martin J. Bligh wrote:
> Oh, I'm being an idiot and looking at the wrong tree. It's __deprecated,
> but I still can't think of a clean way to locally undefine that for
> just EXPORT_SYMBOL.

We could in theory create a new EXPORT_SYMBOL variant that does not
reference the symbol directly. This does a little less compile-time
checks but helps reduce the noise. The big advantage of this
would be that we could once again build kernels with -Werror on
developer machines.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/include/linux/module.h b/include/linux/module.h
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -182,21 +182,26 @@ void *__symbol_get_gpl(const char *symbo
 #endif
 
 /* For every exported symbol, place a struct in the __ksymtab section */
-#define __EXPORT_SYMBOL(sym, sec)				\
-	__CRC_SYMBOL(sym, sec)					\
-	static const char __kstrtab_##sym[]			\
+#define __EXPORT_SYMBOL(name, sym, sec)				\
+	__CRC_SYMBOL(name, sec)					\
+	static const char __kstrtab_##name[]			\
 	__attribute__((section("__ksymtab_strings")))		\
-	= MODULE_SYMBOL_PREFIX #sym;                    	\
-	static const struct kernel_symbol __ksymtab_##sym	\
+	= MODULE_SYMBOL_PREFIX #name;                    	\
+	static const struct kernel_symbol __ksymtab_##name	\
 	__attribute_used__					\
 	__attribute__((section("__ksymtab" sec), unused))	\
-	= { (unsigned long)&sym, __kstrtab_##sym }
+	= { (unsigned long)&sym, __kstrtab_##name }
 
 #define EXPORT_SYMBOL(sym)					\
-	__EXPORT_SYMBOL(sym, "")
+	__EXPORT_SYMBOL(sym, sym, "")
 
 #define EXPORT_SYMBOL_GPL(sym)					\
-	__EXPORT_SYMBOL(sym, "_gpl")
+	__EXPORT_SYMBOL(sym, sym, "_gpl")
+
+#define EXPORT_DEPRECATED_SYMBOL(sym)				\
+	extern void __deprecated_ ## sym 			\
+			__attribute__((alias(#sym)));		\
+	__EXPORT_SYMBOL(sym, __deprecated_ ## sym, "_gpl")
 
 #endif
 
------------------------

Obviously it does not apply today but it does not make sense
to warn about the old API symbol being exported.
Do you see something wrong in the approach Arnd used?

I just never came around to look at it.

	Sam

Re: [PATCH] Permit silencing of __deprecated warnings.

[Arjan van de Ven]

On Thu, 25 Oct 2007 04:06:13 -0400 (EDT)
Jeff Garzik <jeff@garzik.org> wrote:

> The __deprecated marker is quite useful in highlighting the remnants
> of old APIs that want removing.
> 
> However, it is quite normal for one or more years to pass, before the
> (usually ancient, bitrotten) code in question is either updated or
> deleted.
> 
> Thus, like __must_check, add a Kconfig option that permits the
> silencing of this compiler warning.
> 
> This change mimics the ifdef-ery and Kconfig defaults of MUST_CHECK as
> closely as possible.
> 

it would be nice if we could have this always-on for external modules,
so that people who're developing new modules realize from the start
that they're using API's they shouldn't, no matter what config option
they (or their distro) picked...


-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

[PATCH] Remove #warnings for longstanding conditions.

[Jeff Garzik]

These two warnings...

drivers/isdn/capi/capidrv.c:2126:3: warning: #warning FIXME: maybe a
race condition the card should be removed here from global list /kkeil

drivers/scsi/advansys.c:71:2: warning: #warning this driver is still
not properly converted to the DMA API

...represent conditions that have existed for years, and are duly noted
in FIXMEs.  There does not seem to be much need to warn on every kernel
build for a driver bug or handicap that has existed for years.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/isdn/capi/capidrv.c |    3 ++-
 drivers/scsi/advansys.c     |    1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index 476012b..44f954d 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -2123,7 +2123,8 @@ static int capidrv_delcontr(u16 contr)
 		printk(KERN_ERR "capidrv: delcontr: no contr %u\n", contr);
 		return -1;
 	}
-	#warning FIXME: maybe a race condition the card should be removed here from global list /kkeil
+
+	/* FIXME: maybe a race condition the card should be removed here from global list /kkeil */
 	spin_unlock_irqrestore(&global_lock, flags);
 
 	del_timer(&card->listentimer);
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 9dd3952..e13e3a8 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -68,7 +68,6 @@
  *  7. advansys_info is not safe against multiple simultaneous callers
  *  8. Add module_param to override ISA/VLB ioport array
  */
-#warning this driver is still not properly converted to the DMA API
 
 /* Enable driver /proc statistics. */
 #define ADVANSYS_STATS
-- 
1.5.2.4

Re: [PATCH] Remove #warnings for longstanding conditions.

[Karsten Keil]

On Thu, Oct 25, 2007 at 04:06:15AM -0400, Jeff Garzik wrote:
> These two warnings...
> 
> drivers/isdn/capi/capidrv.c:2126:3: warning: #warning FIXME: maybe a
> race condition the card should be removed here from global list /kkeil
> 
> drivers/scsi/advansys.c:71:2: warning: #warning this driver is still
> not properly converted to the DMA API
> 
> ...represent conditions that have existed for years, and are duly noted
> in FIXMEs.  There does not seem to be much need to warn on every kernel
> build for a driver bug or handicap that has existed for years.
> 
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/isdn/capi/capidrv.c |    3 ++-

Acked.

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg)

Re: [PATCH] Remove #warnings for longstanding conditions.

[Matthew Wilcox]

On Thu, Oct 25, 2007 at 04:06:15AM -0400, Jeff Garzik wrote:
> drivers/scsi/advansys.c:71:2: warning: #warning this driver is still
> not properly converted to the DMA API
> 
> ...represent conditions that have existed for years, and are duly noted
> in FIXMEs.  There does not seem to be much need to warn on every kernel
> build for a driver bug or handicap that has existed for years.

I'll be removing this #warning from advansys when I get rid of the
last bus_to_virt.  Which I've already done ... it's just that the
resulting driver works on i386, but crashes parisc, so there's clearly a
bug.  I haven't had time to track it down yet.  So NACK this patch.

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] Remove #warnings for longstanding conditions.

[Jeff Garzik]

Matthew Wilcox wrote:

>> drivers/scsi/advansys.c:71:2: warning: #warning this driver is still
>> not properly converted to the DMA API

> I'll be removing this #warning from advansys when I get rid of the
> last bus_to_virt.  Which I've already done ... it's just that the
> resulting driver works on i386, but crashes parisc, so there's clearly a
> bug.  I haven't had time to track it down yet.  So NACK this patch.

Well, two main questions...

Is this warning of value to anybody but you?
Is this information worth printing out on everyone else's kernel build?

If yes, great!  But it sounds like this warning is analagous to

   #warning willy's TODO list: get milk, buy groceries, read email, ...

which doesn't add much value to others' kernel builds :)

	Jeff

Re: [PATCH] Remove #warnings for longstanding conditions.

[Matthew Wilcox]

On Thu, Oct 25, 2007 at 10:07:17PM -0400, Jeff Garzik wrote:
> Is this warning of value to anybody but you?

Yup.  It signals this driver isn't production quality yet.

> Is this information worth printing out on everyone else's kernel build?

I thnk that's worth noting to anyone who's enabled it.

> If yes, great!  But it sounds like this warning is analagous to
> 
>   #warning willy's TODO list: get milk, buy groceries, read email, ...
> 
> which doesn't add much value to others' kernel builds :)

I didn't add the #warning; it was there before 2.6.12-rc2.  I haven't
added warnings for anything else on the todo list (which remains
mightily long, but I'm acually crossing things off it rather than just
adding to it now).

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [PATCH] Remove #warnings for longstanding conditions.

[Jeff Garzik]

Matthew Wilcox wrote:
> On Thu, Oct 25, 2007 at 10:07:17PM -0400, Jeff Garzik wrote:
>> Is this warning of value to anybody but you?
> 
> Yup.  It signals this driver isn't production quality yet.
> 
>> Is this information worth printing out on everyone else's kernel build?
> 
> I thnk that's worth noting to anyone who's enabled it.

OK, thanks!

	Jeff

[PATCH] ISDN/capidrv: fix casting warning

[Jeff Garzik]

drivers/isdn/capi/capidrv.c: In function 'if_sendbuf':
drivers/isdn/capi/capidrv.c:1865: warning: cast from pointer to integer
of different size

We are passing a kernel pointer, skb->data, but the interface itself is
limited to 32 bits.  A future changeset may want to mark this code
32-bit only, if it turns out cmsg->Data value truncation on 64-bit
platforms is problematic in practice.

Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
 drivers/isdn/capi/capidrv.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
index 44f954d..a4814e1 100644
--- a/drivers/isdn/capi/capidrv.c
+++ b/drivers/isdn/capi/capidrv.c
@@ -1862,7 +1862,7 @@ static int if_sendbuf(int id, int channel, int doack, struct sk_buff *skb)
 	datahandle = nccip->datahandle;
 	capi_fill_DATA_B3_REQ(&sendcmsg, global.ap.applid, card->msgid++,
 			      nccip->ncci,	/* adr */
-			      (u32) skb->data,	/* Data */
+			      (unsigned long) skb->data, /* Data */
 			      skb->len,		/* DataLength */
 			      datahandle,	/* DataHandle */
 			      0	/* Flags */
-- 
1.5.2.4

Re: [PATCH] ISDN/capidrv: fix casting warning

[Karsten Keil]

On Thu, Oct 25, 2007 at 04:06:16AM -0400, Jeff Garzik wrote:
> drivers/isdn/capi/capidrv.c: In function 'if_sendbuf':
> drivers/isdn/capi/capidrv.c:1865: warning: cast from pointer to integer
> of different size
> 
> We are passing a kernel pointer, skb->data, but the interface itself is
> limited to 32 bits.  A future changeset may want to mark this code
> 32-bit only, if it turns out cmsg->Data value truncation on 64-bit
> platforms is problematic in practice.
> 

I think I should really add somewhere a comment for this issue.
This field is not used in practice in linux kernel (neither in 32 or
64 bit), but should have some value, since a CAPI message trace will
display it.
The correct value in the 32 bit case is the address of the
data, in 64 bit it makes no sense, maybe we should use 0 here.

> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
> ---
>  drivers/isdn/capi/capidrv.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/isdn/capi/capidrv.c b/drivers/isdn/capi/capidrv.c
> index 44f954d..a4814e1 100644
> --- a/drivers/isdn/capi/capidrv.c
> +++ b/drivers/isdn/capi/capidrv.c
> @@ -1862,7 +1862,7 @@ static int if_sendbuf(int id, int channel, int doack, struct sk_buff *skb)
>  	datahandle = nccip->datahandle;
>  	capi_fill_DATA_B3_REQ(&sendcmsg, global.ap.applid, card->msgid++,
>  			      nccip->ncci,	/* adr */
> -			      (u32) skb->data,	/* Data */
> +			      (unsigned long) skb->data, /* Data */
>  			      skb->len,		/* DataLength */
>  			      datahandle,	/* DataHandle */
>  			      0	/* Flags */
> -- 
> 1.5.2.4

-- 
Karsten Keil
SuSE Labs
ISDN and VOIP development
SUSE LINUX Products GmbH, Maxfeldstr.5 90409 Nuernberg, GF: Markus Rex, HRB 16746 (AG Nuernberg)

Re: [PATCH] ISDN/capidrv: fix casting warning

[Jeff Garzik]

Karsten Keil wrote:
> On Thu, Oct 25, 2007 at 04:06:16AM -0400, Jeff Garzik wrote:
>> drivers/isdn/capi/capidrv.c: In function 'if_sendbuf':
>> drivers/isdn/capi/capidrv.c:1865: warning: cast from pointer to integer
>> of different size
>>
>> We are passing a kernel pointer, skb->data, but the interface itself is
>> limited to 32 bits.  A future changeset may want to mark this code
>> 32-bit only, if it turns out cmsg->Data value truncation on 64-bit
>> platforms is problematic in practice.
>>
> 
> I think I should really add somewhere a comment for this issue.
> This field is not used in practice in linux kernel (neither in 32 or
> 64 bit), but should have some value, since a CAPI message trace will
> display it.
> The correct value in the 32 bit case is the address of the
> data, in 64 bit it makes no sense, maybe we should use 0 here.

So noted...  I will make sure the next revision of the patch fulfills 
all your wishes :)

	Jeff