Re: [wireless-next:master 82/98] drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)

Re: [wireless-next:master 82/98] drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)

[Eugene Krasnikov]

Hm.. weird. I was sure that i fixed all sparse warnings before
contribution. My sparse does not show this warning. Let me try to
reproduce this error and come back to you. Thanx for you feedback.

On Fri, Oct 11, 2013 at 2:47 AM, Fengguang Wu <fengguang.wu@intel.com> wrote:
>
> Hi Eugene,
>
> FYI, there are new sparse warnings show up in
>
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-next.git master
> head:   e25aa82a89db9e3b09d02b5481aa375c7712a10d
> commit: 8e84c25821698bdef73c0329fb2022a4673b7adc [82/98] wcn36xx: mac80211 driver for Qualcomm WCN3660/WCN3680 hardware
> reproduce: make C=1 CF=-D__CHECK_ENDIAN__
>
>>> drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)
>    drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24:    expected unsigned int [unsigned] [usertype] <noident>
>    drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24:    got restricted __be32 [usertype] <noident>
>>> drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)
>    drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24:    expected unsigned int [unsigned] [usertype] <noident>
>    drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24:    got restricted __be32 [usertype] <noident>
>
> vim +88 drivers/net/wireless/ath/wcn36xx/wcn36xx.h
>
> 8e84c258 Eugene Krasnikov 2013-10-08  72                print_hex_dump(KERN_DEBUG, pr_fmt(prefix_str),  \
> 8e84c258 Eugene Krasnikov 2013-10-08  73                               DUMP_PREFIX_OFFSET, 32, 1,       \
> 8e84c258 Eugene Krasnikov 2013-10-08  74                               buf, len, false);                \
> 8e84c258 Eugene Krasnikov 2013-10-08  75  } while (0)
> 8e84c258 Eugene Krasnikov 2013-10-08  76
> 8e84c258 Eugene Krasnikov 2013-10-08  77  #define WCN36XX_HW_CHANNEL(__wcn) (__wcn->hw->conf.chandef.chan->hw_value)
> 8e84c258 Eugene Krasnikov 2013-10-08  78  #define WCN36XX_BAND(__wcn) (__wcn->hw->conf.chandef.chan->band)
> 8e84c258 Eugene Krasnikov 2013-10-08  79  #define WCN36XX_CENTER_FREQ(__wcn) (__wcn->hw->conf.chandef.chan->center_freq)
> 8e84c258 Eugene Krasnikov 2013-10-08  80  #define WCN36XX_LISTEN_INTERVAL(__wcn) (__wcn->hw->conf.listen_interval)
> 8e84c258 Eugene Krasnikov 2013-10-08  81  #define WCN36XX_FLAGS(__wcn) (__wcn->hw->flags)
> 8e84c258 Eugene Krasnikov 2013-10-08  82  #define WCN36XX_MAX_POWER(__wcn) (__wcn->hw->conf.chandef.chan->max_power)
> 8e84c258 Eugene Krasnikov 2013-10-08  83
> 8e84c258 Eugene Krasnikov 2013-10-08  84  static inline void buff_to_be(u32 *buf, size_t len)
> 8e84c258 Eugene Krasnikov 2013-10-08  85  {
> 8e84c258 Eugene Krasnikov 2013-10-08  86        int i;
> 8e84c258 Eugene Krasnikov 2013-10-08  87        for (i = 0; i < len; i++)
> 8e84c258 Eugene Krasnikov 2013-10-08 @88                buf[i] = cpu_to_be32(buf[i]);
> 8e84c258 Eugene Krasnikov 2013-10-08  89  }
> 8e84c258 Eugene Krasnikov 2013-10-08  90
> 8e84c258 Eugene Krasnikov 2013-10-08  91  struct nv_data {
> 8e84c258 Eugene Krasnikov 2013-10-08  92        int     is_valid;
> 8e84c258 Eugene Krasnikov 2013-10-08  93        u8      table;
> 8e84c258 Eugene Krasnikov 2013-10-08  94  };
> 8e84c258 Eugene Krasnikov 2013-10-08  95
> 8e84c258 Eugene Krasnikov 2013-10-08  96  /* Interface for platform control path
>
> ---
> 0-DAY kernel build testing backend              Open Source Technology Center
> http://lists.01.org/mailman/listinfo/kbuild                 Intel Corporation



-- 
Best regards,
Eugene

Re: [wireless-next:master 82/98] drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)

[Johannes Berg]

On Fri, 2013-10-11 at 08:19 +0100, Eugene Krasnikov wrote:
> Hm.. weird. I was sure that i fixed all sparse warnings before
> contribution. My sparse does not show this warning. Let me try to
> reproduce this error and come back to you. Thanx for you feedback.

> > reproduce: make C=1 CF=-D__CHECK_ENDIAN__

Re: [wireless-next:master 82/98] drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)

[Eugene Krasnikov]

Hi Johannes,

Do you see the same warning? With the latest sparse i do not see this
warning. I assume this warning depends on sparse version since with my
environment it does not pop up. Also from code perspective it looks
fine. Any ideas?:)

On Fri, Oct 11, 2013 at 8:24 AM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2013-10-11 at 08:19 +0100, Eugene Krasnikov wrote:
>> Hm.. weird. I was sure that i fixed all sparse warnings before
>> contribution. My sparse does not show this warning. Let me try to
>> reproduce this error and come back to you. Thanx for you feedback.
>
>> > reproduce: make C=1 CF=-D__CHECK_ENDIAN__
>
>
>



-- 
Best regards,
Eugene

Re: [wireless-next:master 82/98] drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)

[Johannes Berg]

On Fri, 2013-10-11 at 21:39 +0100, Eugene Krasnikov wrote:

> Do you see the same warning? 

Yes

> With the latest sparse i do not see this
> warning. 

$ sparse --version
v0.4.4-133-g5449cfb

> I assume this warning depends on sparse version since with my
> environment it does not pop up. Also from code perspective it looks
> fine. Any ideas?:)

Hard to believe - did you pass the CF= on the command line? I suggest

ccflags-y += -D__CHECK_ENDIAN__

in your Makefile.

Also, code might be fine, but buff_to_be() is still not a good idea
since it breaks all endian annotation.

Also, I suggested to run smatch, I guess you never did:
drivers/net/wireless/ath/wcn36xx/smd.c:1314 wcn36xx_smd_send_beacon() warn: inconsistent returns mutex:&wcn->hal_mutex: locked (1289 [(-12)]) unlocked (1314 [0], 1314 [s32min-(-1),1-s32max])
drivers/net/wireless/ath/wcn36xx/smd.c:1357 wcn36xx_smd_update_proberesp_tmpl() warn: inconsistent returns mutex:&wcn->hal_mutex: locked (1330 [(-7)]) unlocked (1357 [0], 1357 [s32min-(-1),1-s32max])
drivers/net/wireless/ath/wcn36xx/smd.c:1626 wcn36xx_smd_keep_alive_req() warn: inconsistent returns mutex:&wcn->hal_mutex: locked (1609 [(-22)]) unlocked (1626 [0], 1626 [s32min-(-1),1-s32max])
drivers/net/wireless/ath/wcn36xx/smd.c:2041 wcn36xx_smd_rsp_process() error: potential null dereference 'msg_ind'.  (kmalloc returns null)

johannes

Re: [wireless-next:master 82/98] drivers/net/wireless/ath/wcn36xx/wcn36xx.h:88:24: sparse: incorrect type in assignment (different base types)

[Eugene Krasnikov]

> $ sparse --version
> v0.4.4-133-g5449cfb

My is: sparse --version
v0.4.5-rc1


> Hard to believe - did you pass the CF= on the command line? I suggest
>
> ccflags-y += -D__CHECK_ENDIAN__

Agree that it's not a version difference :) Yes , i set check endian.
Weird, will try to modify makefile and see if that helps.


> Also, code might be fine, but buff_to_be() is still not a good idea
> since it breaks all endian annotation.

Could you please explain in more details how does buff_to_be break
endian annotation?


> Also, I suggested to run smatch, I guess you never did:

I ran it and even fixed some of the errors. These errors i might just
missed because of the output is flooded with messages like:
drivers/net/wireless/ath/wcn36xx/main.c:24 (null)() info: ignoring
unreachable code.

Thanx for pointing to these warnings.

On Sat, Oct 12, 2013 at 7:25 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Fri, 2013-10-11 at 21:39 +0100, Eugene Krasnikov wrote:
>
>> Do you see the same warning?
>
> Yes
>
>> With the latest sparse i do not see this
>> warning.
>
> $ sparse --version
> v0.4.4-133-g5449cfb
>
>> I assume this warning depends on sparse version since with my
>> environment it does not pop up. Also from code perspective it looks
>> fine. Any ideas?:)
>
> Hard to believe - did you pass the CF= on the command line? I suggest
>
> ccflags-y += -D__CHECK_ENDIAN__
>
> in your Makefile.
>
> Also, code might be fine, but buff_to_be() is still not a good idea
> since it breaks all endian annotation.
>
> Also, I suggested to run smatch, I guess you never did:
> drivers/net/wireless/ath/wcn36xx/smd.c:1314 wcn36xx_smd_send_beacon() warn: inconsistent returns mutex:&wcn->hal_mutex: locked (1289 [(-12)]) unlocked (1314 [0], 1314 [s32min-(-1),1-s32max])
> drivers/net/wireless/ath/wcn36xx/smd.c:1357 wcn36xx_smd_update_proberesp_tmpl() warn: inconsistent returns mutex:&wcn->hal_mutex: locked (1330 [(-7)]) unlocked (1357 [0], 1357 [s32min-(-1),1-s32max])
> drivers/net/wireless/ath/wcn36xx/smd.c:1626 wcn36xx_smd_keep_alive_req() warn: inconsistent returns mutex:&wcn->hal_mutex: locked (1609 [(-22)]) unlocked (1626 [0], 1626 [s32min-(-1),1-s32max])
> drivers/net/wireless/ath/wcn36xx/smd.c:2041 wcn36xx_smd_rsp_process() error: potential null dereference 'msg_ind'.  (kmalloc returns null)
>
> johannes
>



-- 
Best regards,
Eugene