[PATCH] zd1211rw: make CR_INTERRUPT cast explicit

[PATCH] zd1211rw: make CR_INTERRUPT cast explicit

[John W. Linville]

  CHECK   drivers/net/wireless/zd1211rw/zd_usb.c
drivers/net/wireless/zd1211rw/zd_usb.c:376:24: warning: implicit cast from nocast type

The value comparison is intentional, so forcing the cast seems warranted
in order to silence the sparse warning.

Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/wireless/zd1211rw/zd_usb.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
index 818e148..acbf4d9 100644
--- a/drivers/net/wireless/zd1211rw/zd_usb.c
+++ b/drivers/net/wireless/zd1211rw/zd_usb.c
@@ -373,7 +373,7 @@ static inline void handle_regs_int(struct urb *urb)
 	spin_lock(&intr->lock);
 
 	int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
-	if (int_num == CR_INTERRUPT) {
+	if (int_num == (u16 __force) CR_INTERRUPT) {
 		struct zd_mac *mac = zd_hw_mac(zd_usb_to_hw(urb->context));
 		memcpy(&mac->intr_buffer, urb->transfer_buffer,
 				USB_MAX_EP_INT_BUFFER);
-- 
1.7.1.1

Re: [PATCH] zd1211rw: make CR_INTERRUPT cast explicit

[Pavel Roskin]

On Wed, 2010-07-21 at 11:08 -0400, John W. Linville wrote:
> CHECK   drivers/net/wireless/zd1211rw/zd_usb.c
> drivers/net/wireless/zd1211rw/zd_usb.c:376:24: warning: implicit cast from nocast type
> 
> The value comparison is intentional, so forcing the cast seems warranted
> in order to silence the sparse warning.
> 
> Signed-off-by: John W. Linville <linville@tuxdriver.com>
> ---
>  drivers/net/wireless/zd1211rw/zd_usb.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
> index 818e148..acbf4d9 100644
> --- a/drivers/net/wireless/zd1211rw/zd_usb.c
> +++ b/drivers/net/wireless/zd1211rw/zd_usb.c
> @@ -373,7 +373,7 @@ static inline void handle_regs_int(struct urb *urb)
>  	spin_lock(&intr->lock);
>  
>  	int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
> -	if (int_num == CR_INTERRUPT) {
> +	if (int_num == (u16 __force) CR_INTERRUPT) {

I think __force should be used sparingly, especially outside headers.  I
tried other ways to prevent the warning.  I tried declaring int_num as
zd_addr_t, but sparse keeps complaining about the same comparison.

I can rewrite "if" as "switch" to suppress the warning, but it makes the
code less readable.  I think it's a bug in sparse.  There should be no
casts when comparing values of the same type.

I understand the benefits of fixing sparse warnings, but this patch
works around a sparse bug and sets a bad example.

-- 
Regards,
Pavel Roskin

Re: [PATCH] zd1211rw: make CR_INTERRUPT cast explicit

[John W. Linville]

On Wed, Jul 21, 2010 at 12:36:58PM -0400, Pavel Roskin wrote:
> On Wed, 2010-07-21 at 11:08 -0400, John W. Linville wrote:
> > CHECK   drivers/net/wireless/zd1211rw/zd_usb.c
> > drivers/net/wireless/zd1211rw/zd_usb.c:376:24: warning: implicit cast from nocast type
> > 
> > The value comparison is intentional, so forcing the cast seems warranted
> > in order to silence the sparse warning.
> > 
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > ---
> >  drivers/net/wireless/zd1211rw/zd_usb.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/zd1211rw/zd_usb.c b/drivers/net/wireless/zd1211rw/zd_usb.c
> > index 818e148..acbf4d9 100644
> > --- a/drivers/net/wireless/zd1211rw/zd_usb.c
> > +++ b/drivers/net/wireless/zd1211rw/zd_usb.c
> > @@ -373,7 +373,7 @@ static inline void handle_regs_int(struct urb *urb)
> >  	spin_lock(&intr->lock);
> >  
> >  	int_num = le16_to_cpu(*(__le16 *)(urb->transfer_buffer+2));
> > -	if (int_num == CR_INTERRUPT) {
> > +	if (int_num == (u16 __force) CR_INTERRUPT) {
> 
> I think __force should be used sparingly, especially outside headers.  I
> tried other ways to prevent the warning.  I tried declaring int_num as
> zd_addr_t, but sparse keeps complaining about the same comparison.

Actually, it looks like it works w/o the "__force" as well.
Any objection to a version like that?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [PATCH] zd1211rw: make CR_INTERRUPT cast explicit

[Pavel Roskin]

On Thu, 2010-07-22 at 15:03 -0400, John W. Linville wrote:
> On Wed, Jul 21, 2010 at 12:36:58PM -0400, Pavel Roskin wrote:
> > I think __force should be used sparingly, especially outside headers.  I
> > tried other ways to prevent the warning.  I tried declaring int_num as
> > zd_addr_t, but sparse keeps complaining about the same comparison.
> 
> Actually, it looks like it works w/o the "__force" as well.
> Any objection to a version like that?

Ideally, I would avoid any workarounds for sparse.  The bug was
acknowledged by a sparse developer, so I expect it to be fixed.
http://www.spinics.net/lists/linux-sparse/msg02167.html

However, a version without __force would be more acceptable than a
version with __force.

-- 
Regards,
Pavel Roskin