On Thu, Sep 17, 2015 at 06:14:04AM -0700, Greg Kroah-Hartman wrote:
> On Thu, Sep 17, 2015 at 01:48:14PM +0800, Navy Cheng wrote:
> > On Wed, Sep 16, 2015 at 09:52:06PM -0700, Greg Kroah-Hartman wrote:
> > > On Wed, Sep 16, 2015 at 09:06:13PM +0800, Navy Cheng wrote:
> > > > The type of value is u16 however the return type of cpu_to_le16() is
> > > > __le16. The incorrect type of assignment is complained by sparse.
> > > >
> > > > Signed-off-by: Navy Cheng <navych@xxxxxxx>
> > > > ---
> > > > drivers/staging/wlan-ng/hfa384x.h | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/wlan-ng/hfa384x.h b/drivers/staging/wlan-ng/hfa384x.h
> > > > index 8dfe438..fe7df37 100644
> > > > --- a/drivers/staging/wlan-ng/hfa384x.h
> > > > +++ b/drivers/staging/wlan-ng/hfa384x.h
> > > > @@ -1385,7 +1385,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val)
> > > >
> > > > static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val)
> > > > {
> > > > - u16 value = cpu_to_le16(val);
> > > > + __le16 value = cpu_to_le16(val);
> > > >
> > > > return hfa384x_drvr_setconfig(hw, rid, &value, sizeof(value));
> > >
> > > but value is a pointer to a u16, doesn't this change now cause another
> > > sparse warning?
> >
> > Sorry, do you mean &value is a pointer to a u16?
>
> Yes.
>
> > I use *make C=2 CF="-D__CHECK_ENDIAN__" drivers/staging/wlan-ng* to check
> > the warnings.
> >
> > before the fixing, there are many repeated warnings like this:
> >
> > /home/navy/linux/drivers/staging/wlan-ng/hfa384x.h:1382:34: warning: cast to restricted __le16
> > /home/navy/linux/drivers/staging/wlan-ng/hfa384x.h:1388:21: warning: incorrect type in initializer (different base types)
> > /home/navy/linux/drivers/staging/wlan-ng/hfa384x.h:1388:21: expected unsigned short [unsigned] [usertype] value
> > /home/navy/linux/drivers/staging/wlan-ng/hfa384x.h:1388:21: got restricted __le16 [usertype] <noident>
> >
> > after the fixing, the warnigs of hfa384x.h is still redundant, but only
> > one pattern, like:
> >
> > /home/navy/linux/drivers/staging/wlan-ng/hfa384x.h:1382:34: warning: cast to restricted __le16
> >
> > So I think this fixing dosn't cause another warnings.
>
> But logically it isn't the right thing to do, right?
Do you mean the change of &value's type will cause any problem? And the
problem is in this code?
return hfa384x_drvr_setconfig(hw, rid, &value, sizeof(value));
The definition of hfa384x_drvr_setconfig() is below, and buf is a pointer
to void.
int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void *buf, u16 len)
{
return hfa384x_dowrid_wait(hw, rid, buf, len);
}
1) when the type of value is u16, &value is passed to hfa384x_drvr_setconfig()
as (u16 *) other than (void *) without any warning. Why no warnings?
2) when the type of value is __le16, &value is treated as (void *) by
hfa384x_drvr_setconfig(), like 1). How to deal with (void *)&value is the
problem of hfa384x_drvr_setconfig(). So I think __le16 is the same as u16
when passed to hfa384x_drvr_setconfig(). Am I right?
>
> > There is still one warnings for hfa384x.h and many other warnings for other
> > files in staging/wlan-ng/. I'm not familiar with sending patch sets, so I
> > only send one patch about one problem.
> >
> > Do this patch is needed and do you want a patchv2? If the warnings is
> > needed, I want to send more patches to fix the sparse warings in wlan-ng.
>
> Don't blindly silence warnings, look at the code and verify it is
> working properly with your changes.
>
> thanks,
>
> greg k-h
Thanks for your advice.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
