* [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings @ 2021-06-03 8:24 Wan Jiabing 2021-06-03 8:36 ` Jiabing Wan 2021-06-03 9:18 ` Greg Kroah-Hartman 0 siblings, 2 replies; 7+ messages in thread From: Wan Jiabing @ 2021-06-03 8:24 UTC (permalink / raw) To: Greg Kroah-Hartman, Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel Cc: Wan Jiabing Fix some "incorrect type in assignment" in rtw_security.c. The sparse warings: drivers/staging//rtl8723bs/core/rtw_security.c:72:50: warning: incorrect type in assignment drivers/staging//rtl8723bs/core/rtw_security.c:72:50: expected restricted __le32 [usertype] drivers/staging//rtl8723bs/core/rtw_security.c:72:50: got unsigned int drivers/staging//rtl8723bs/core/rtw_security.c:80:50: warning: incorrect type in assignment drivers/staging//rtl8723bs/core/rtw_security.c:80:50: expected restricted __le32 [usertype] drivers/staging//rtl8723bs/core/rtw_security.c:80:50: got unsigned int drivers/staging//rtl8723bs/core/rtw_security.c:124:33: warning: cast to restricted __le32 drivers/staging//rtl8723bs/core/rtw_security.c:509:58: warning: incorrect type in assignment drivers/staging//rtl8723bs/core/rtw_security.c:509:58: expected restricted __le32 [usertype] drivers/staging//rtl8723bs/core/rtw_security.c:509:58: got unsigned int drivers/staging//rtl8723bs/core/rtw_security.c:517:58: warning: incorrect type in assignment drivers/staging//rtl8723bs/core/rtw_security.c:517:58: expected restricted __le32 [usertype] drivers/staging//rtl8723bs/core/rtw_security.c:517:58: got unsigned int drivers/staging//rtl8723bs/core/rtw_security.c:621:41: warning: cast to restricted __le32 Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> --- drivers/staging/rtl8723bs/core/rtw_security.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c index a99f439..4760999 100644 --- a/drivers/staging/rtl8723bs/core/rtw_security.c +++ b/drivers/staging/rtl8723bs/core/rtw_security.c @@ -36,7 +36,7 @@ const char *security_type_str(u8 value) void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) { /* exclude ICV */ - unsigned char crc[4]; + u8 crc[4]; signed int curfragnum, length; u32 keylength; @@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); arc4_setkey(ctx, wepkey, 3 + keylength); arc4_crypt(ctx, payload, payload, length); @@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) } else { length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); arc4_setkey(ctx, wepkey, 3 + keylength); arc4_crypt(ctx, payload, payload, length); arc4_crypt(ctx, payload + length, crc, 4); @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) arc4_crypt(ctx, payload, payload, length); /* calculate icv and compare the icv */ - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); } } @@ -506,7 +506,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) if ((curfragnum+1) == pattrib->nr_frags) { /* 4 the last fragment */ length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); @@ -514,7 +514,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) } else { length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); @@ -618,7 +618,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe) arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re:[PATCH] staging: rtl8723bs: core: fix some incorrect type warnings 2021-06-03 8:24 [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings Wan Jiabing @ 2021-06-03 8:36 ` Jiabing Wan 2021-06-03 9:18 ` Greg Kroah-Hartman 1 sibling, 0 replies; 7+ messages in thread From: Jiabing Wan @ 2021-06-03 8:36 UTC (permalink / raw) To: Wan Jiabing, Greg Kroah-Hartman, Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel Hi,all I fix these warnings by refering other patches. Actually, I don't understand clearly what I did. But there are no warnings after this fix. So if this fix were totally wrong, please told me and gave me some suggestions. It is important for me to fix a sparse bug. Thanks, Jiabing >Fix some "incorrect type in assignment" in rtw_security.c. > >The sparse warings: >drivers/staging//rtl8723bs/core/rtw_security.c:72:50: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:72:50: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:72:50: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:80:50: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:80:50: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:80:50: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:124:33: warning: cast to restricted __le32 >drivers/staging//rtl8723bs/core/rtw_security.c:509:58: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:509:58: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:509:58: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:517:58: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:517:58: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:517:58: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:621:41: warning: cast to restricted __le32 > >Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> >--- > drivers/staging/rtl8723bs/core/rtw_security.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > >diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c >index a99f439..4760999 100644 >--- a/drivers/staging/rtl8723bs/core/rtw_security.c >+++ b/drivers/staging/rtl8723bs/core/rtw_security.c >@@ -36,7 +36,7 @@ const char *security_type_str(u8 value) > void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > { /* exclude ICV */ > >- unsigned char crc[4]; >+ u8 crc[4]; > > signed int curfragnum, length; > u32 keylength; >@@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > > length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; > >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > > arc4_setkey(ctx, wepkey, 3 + keylength); > arc4_crypt(ctx, payload, payload, length); >@@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > > } else { > length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > arc4_setkey(ctx, wepkey, 3 + keylength); > arc4_crypt(ctx, payload, payload, length); > arc4_crypt(ctx, payload + length, crc, 4); >@@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) > arc4_crypt(ctx, payload, payload, length); > > /* calculate icv and compare the icv */ >- *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); > > } > } >@@ -506,7 +506,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) > > if ((curfragnum+1) == pattrib->nr_frags) { /* 4 the last fragment */ > length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > > arc4_setkey(ctx, rc4key, 16); > arc4_crypt(ctx, payload, payload, length); >@@ -514,7 +514,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) > > } else { > length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > > arc4_setkey(ctx, rc4key, 16); > arc4_crypt(ctx, payload, payload, length); >@@ -618,7 +618,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe) > arc4_setkey(ctx, rc4key, 16); > arc4_crypt(ctx, payload, payload, length); > >- *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); > > if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || > crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) >-- >2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:[PATCH] staging: rtl8723bs: core: fix some incorrect type warnings @ 2021-06-03 8:36 ` Jiabing Wan 0 siblings, 0 replies; 7+ messages in thread From: Jiabing Wan @ 2021-06-03 8:36 UTC (permalink / raw) To: Wan Jiabing, Greg Kroah-Hartman, Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel Hi,all I fix these warnings by refering other patches. Actually, I don't understand clearly what I did. But there are no warnings after this fix. So if this fix were totally wrong, please told me and gave me some suggestions. It is important for me to fix a sparse bug. Thanks, Jiabing >Fix some "incorrect type in assignment" in rtw_security.c. > >The sparse warings: >drivers/staging//rtl8723bs/core/rtw_security.c:72:50: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:72:50: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:72:50: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:80:50: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:80:50: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:80:50: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:124:33: warning: cast to restricted __le32 >drivers/staging//rtl8723bs/core/rtw_security.c:509:58: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:509:58: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:509:58: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:517:58: warning: incorrect type in assignment >drivers/staging//rtl8723bs/core/rtw_security.c:517:58: expected restricted __le32 [usertype] >drivers/staging//rtl8723bs/core/rtw_security.c:517:58: got unsigned int >drivers/staging//rtl8723bs/core/rtw_security.c:621:41: warning: cast to restricted __le32 > >Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> >--- > drivers/staging/rtl8723bs/core/rtw_security.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > >diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c >index a99f439..4760999 100644 >--- a/drivers/staging/rtl8723bs/core/rtw_security.c >+++ b/drivers/staging/rtl8723bs/core/rtw_security.c >@@ -36,7 +36,7 @@ const char *security_type_str(u8 value) > void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > { /* exclude ICV */ > >- unsigned char crc[4]; >+ u8 crc[4]; > > signed int curfragnum, length; > u32 keylength; >@@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > > length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; > >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > > arc4_setkey(ctx, wepkey, 3 + keylength); > arc4_crypt(ctx, payload, payload, length); >@@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > > } else { > length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > arc4_setkey(ctx, wepkey, 3 + keylength); > arc4_crypt(ctx, payload, payload, length); > arc4_crypt(ctx, payload + length, crc, 4); >@@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) > arc4_crypt(ctx, payload, payload, length); > > /* calculate icv and compare the icv */ >- *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); > > } > } >@@ -506,7 +506,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) > > if ((curfragnum+1) == pattrib->nr_frags) { /* 4 the last fragment */ > length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > > arc4_setkey(ctx, rc4key, 16); > arc4_crypt(ctx, payload, payload, length); >@@ -514,7 +514,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) > > } else { > length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >- *((__le32 *)crc) = ~crc32_le(~0, payload, length); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > > arc4_setkey(ctx, rc4key, 16); > arc4_crypt(ctx, payload, payload, length); >@@ -618,7 +618,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe) > arc4_setkey(ctx, rc4key, 16); > arc4_crypt(ctx, payload, payload, length); > >- *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); >+ *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); > > if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || > crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) >-- >2.7.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings 2021-06-03 8:36 ` Jiabing Wan (?) @ 2021-06-03 9:17 ` Greg Kroah-Hartman -1 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-06-03 9:17 UTC (permalink / raw) To: Jiabing Wan Cc: Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel On Thu, Jun 03, 2021 at 04:36:55PM +0800, Jiabing Wan wrote: > > > Hi,all > > I fix these warnings by refering other patches. > > Actually, I don't understand clearly what I did. > > But there are no warnings after this fix. > > So if this fix were totally wrong, please told me and > gave me some suggestions. > > It is important for me to fix a sparse bug. Do not fix sparse bugs just for the sake of fixing sparse bugs. Fix things you know are needed to be fixed. Please look closer at this to ensure that you really are fixing this correctly and understand what you are doing before sending it out. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings 2021-06-03 8:24 [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings Wan Jiabing 2021-06-03 8:36 ` Jiabing Wan @ 2021-06-03 9:18 ` Greg Kroah-Hartman 2021-06-04 2:19 ` Jiabing Wan 1 sibling, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2021-06-03 9:18 UTC (permalink / raw) To: Wan Jiabing Cc: Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel On Thu, Jun 03, 2021 at 04:24:59PM +0800, Wan Jiabing wrote: > Fix some "incorrect type in assignment" in rtw_security.c. > > The sparse warings: > drivers/staging//rtl8723bs/core/rtw_security.c:72:50: warning: incorrect type in assignment > drivers/staging//rtl8723bs/core/rtw_security.c:72:50: expected restricted __le32 [usertype] > drivers/staging//rtl8723bs/core/rtw_security.c:72:50: got unsigned int > drivers/staging//rtl8723bs/core/rtw_security.c:80:50: warning: incorrect type in assignment > drivers/staging//rtl8723bs/core/rtw_security.c:80:50: expected restricted __le32 [usertype] > drivers/staging//rtl8723bs/core/rtw_security.c:80:50: got unsigned int > drivers/staging//rtl8723bs/core/rtw_security.c:124:33: warning: cast to restricted __le32 > drivers/staging//rtl8723bs/core/rtw_security.c:509:58: warning: incorrect type in assignment > drivers/staging//rtl8723bs/core/rtw_security.c:509:58: expected restricted __le32 [usertype] > drivers/staging//rtl8723bs/core/rtw_security.c:509:58: got unsigned int > drivers/staging//rtl8723bs/core/rtw_security.c:517:58: warning: incorrect type in assignment > drivers/staging//rtl8723bs/core/rtw_security.c:517:58: expected restricted __le32 [usertype] > drivers/staging//rtl8723bs/core/rtw_security.c:517:58: got unsigned int > drivers/staging//rtl8723bs/core/rtw_security.c:621:41: warning: cast to restricted __le32 > > Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> > --- > drivers/staging/rtl8723bs/core/rtw_security.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c > index a99f439..4760999 100644 > --- a/drivers/staging/rtl8723bs/core/rtw_security.c > +++ b/drivers/staging/rtl8723bs/core/rtw_security.c > @@ -36,7 +36,7 @@ const char *security_type_str(u8 value) > void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > { /* exclude ICV */ > > - unsigned char crc[4]; > + u8 crc[4]; Why change this? > > signed int curfragnum, length; > u32 keylength; > @@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > > length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; > > - *((__le32 *)crc) = ~crc32_le(~0, payload, length); > + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); Are you sure this does what you think it does? What exactly is this doing now? > > arc4_setkey(ctx, wepkey, 3 + keylength); > arc4_crypt(ctx, payload, payload, length); > @@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) > > } else { > length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; > - *((__le32 *)crc) = ~crc32_le(~0, payload, length); > + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > arc4_setkey(ctx, wepkey, 3 + keylength); > arc4_crypt(ctx, payload, payload, length); > arc4_crypt(ctx, payload + length, crc, 4); > @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) > arc4_crypt(ctx, payload, payload, length); > > /* calculate icv and compare the icv */ > - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); > + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); This odd casting feels wrong, why is it correct now? thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re: [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings 2021-06-03 9:18 ` Greg Kroah-Hartman @ 2021-06-04 2:19 ` Jiabing Wan 0 siblings, 0 replies; 7+ messages in thread From: Jiabing Wan @ 2021-06-04 2:19 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel Hi, Greg I feel so sorry for a uncertain patch. I'll learn closer and fix it. After learning deeper, I think: >On Thu, Jun 03, 2021 at 04:24:59PM +0800, Wan Jiabing wrote: >> Fix some "incorrect type in assignment" in rtw_security.c. >> >> The sparse warings: >> drivers/staging//rtl8723bs/core/rtw_security.c:72:50: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:72:50: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:72:50: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:80:50: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:80:50: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:80:50: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:124:33: warning: cast to restricted __le32 >> drivers/staging//rtl8723bs/core/rtw_security.c:509:58: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:509:58: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:509:58: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:517:58: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:517:58: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:517:58: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:621:41: warning: cast to restricted __le32 >> >> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> >> --- >> drivers/staging/rtl8723bs/core/rtw_security.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c >> index a99f439..4760999 100644 >> --- a/drivers/staging/rtl8723bs/core/rtw_security.c >> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c >> @@ -36,7 +36,7 @@ const char *security_type_str(u8 value) >> void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) >> { /* exclude ICV */ >> >> - unsigned char crc[4]; >> + u8 crc[4]; > >Why change this? > * First reason is that other functions which using "~crc32_le" are all declaring "u8 crc[4];". Only this function called "rtw_wep_encrypt" uses "unsigned char crc[4];" to declare crc. But anothor function called "rtw_wep_decrypt" uses "u8 crc[4];" to declare. I think it is confusing and the declaration should be unanimous. * Second reason is that function "crc32_le(~0, payload, length);" returns u32, so u8[4] is described u32. * Third reason is that later function called "arc4_crypt" using "const *u8" as parameter. So I think this change is reasonable. >> >> signed int curfragnum, length; >> u32 keylength; >> @@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) >> >> length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >> >> - *((__le32 *)crc) = ~crc32_le(~0, payload, length); >> + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > >Are you sure this does what you think it does? > >What exactly is this doing now? This change might be wrong totally. I feel sorry for this. arc4_crypt need "const *u8" as parameter, so crc should be type "const *u8". But it use "__le32", so in my opinion, it should be *((u32 *)crc) = ~crc32_le(~0, payload, length); > >> >> arc4_setkey(ctx, wepkey, 3 + keylength); >> arc4_crypt(ctx, payload, payload, length); >> @@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) >> >> } else { >> length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >> - *((__le32 *)crc) = ~crc32_le(~0, payload, length); >> + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); >> arc4_setkey(ctx, wepkey, 3 + keylength); >> arc4_crypt(ctx, payload, payload, length); >> arc4_crypt(ctx, payload + length, crc, 4); >> @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) >> arc4_crypt(ctx, payload, payload, length); >> >> /* calculate icv and compare the icv */ >> - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); >> + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); > >This odd casting feels wrong, why is it correct now? > If we want to keep cpu value, we should fix it like this: *((u32 *)crc) =~crc32_le(~0, payload, length - 4); If we want to keep le32 value, we should fix it like this: *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); These fix are all solve the warnings. The problem is which one should I choose to meet the author's wishes. And in this situation, it is hard to choose which one is better, as after this line, the function return directly. >thanks, > >greg k-h The new fix I choose to meet the author's wishes and fix warnings is following: (Following content is just asked for suggestions and if it were proper, I would send v2.) @@ -36,7 +36,7 @@ const char *security_type_str(u8 value) void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) { /* exclude ICV */ - unsigned char crc[4]; + u8 crc[4]; signed int curfragnum, length; u32 keylength; @@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) =~crc32_le(~0, payload, length); arc4_setkey(ctx, wepkey, 3 + keylength); arc4_crypt(ctx, payload, payload, length); @@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) } else { length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) = ~crc32_le(~0, payload, length); arc4_setkey(ctx, wepkey, 3 + keylength); arc4_crypt(ctx, payload, payload, length); arc4_crypt(ctx, payload + length, crc, 4); @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) arc4_crypt(ctx, payload, payload, length); /* calculate icv and compare the icv */ - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); *- or *- *- + *((u32 *)crc) = ~crc32_le(~0, payload, length - 4); *- *- // !! This code is confusing, because after this line of code, the function return directly. I don't know what is the author's wish. } } @@ -506,7 +506,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) if ((curfragnum+1) == pattrib->nr_frags) { /* 4 the last fragment */ length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) = ~crc32_le(~0, payload, length); arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); @@ -514,7 +514,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) } else { length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) = ~crc32_le(~0, payload, length); arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); @@ -618,7 +618,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe) arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); + *((u32 *)crc) = ~crc32_le(~0, payload, length - 4); * or * * + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); * * //!! This fix also confused me. But, IMO, u32 is better to meet the author's wishes. if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) -- 2.7.4 I think new patch is better, ;). Please review. Thanks, Jiabing ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re:Re: [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings @ 2021-06-04 2:19 ` Jiabing Wan 0 siblings, 0 replies; 7+ messages in thread From: Jiabing Wan @ 2021-06-04 2:19 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Fabio Aiuto, Ross Schmidt, Qiang Ma, Marco Cesati, linux-staging, linux-kernel Hi, Greg I feel so sorry for a uncertain patch. I'll learn closer and fix it. After learning deeper, I think: >On Thu, Jun 03, 2021 at 04:24:59PM +0800, Wan Jiabing wrote: >> Fix some "incorrect type in assignment" in rtw_security.c. >> >> The sparse warings: >> drivers/staging//rtl8723bs/core/rtw_security.c:72:50: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:72:50: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:72:50: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:80:50: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:80:50: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:80:50: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:124:33: warning: cast to restricted __le32 >> drivers/staging//rtl8723bs/core/rtw_security.c:509:58: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:509:58: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:509:58: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:517:58: warning: incorrect type in assignment >> drivers/staging//rtl8723bs/core/rtw_security.c:517:58: expected restricted __le32 [usertype] >> drivers/staging//rtl8723bs/core/rtw_security.c:517:58: got unsigned int >> drivers/staging//rtl8723bs/core/rtw_security.c:621:41: warning: cast to restricted __le32 >> >> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com> >> --- >> drivers/staging/rtl8723bs/core/rtw_security.c | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/staging/rtl8723bs/core/rtw_security.c b/drivers/staging/rtl8723bs/core/rtw_security.c >> index a99f439..4760999 100644 >> --- a/drivers/staging/rtl8723bs/core/rtw_security.c >> +++ b/drivers/staging/rtl8723bs/core/rtw_security.c >> @@ -36,7 +36,7 @@ const char *security_type_str(u8 value) >> void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) >> { /* exclude ICV */ >> >> - unsigned char crc[4]; >> + u8 crc[4]; > >Why change this? > * First reason is that other functions which using "~crc32_le" are all declaring "u8 crc[4];". Only this function called "rtw_wep_encrypt" uses "unsigned char crc[4];" to declare crc. But anothor function called "rtw_wep_decrypt" uses "u8 crc[4];" to declare. I think it is confusing and the declaration should be unanimous. * Second reason is that function "crc32_le(~0, payload, length);" returns u32, so u8[4] is described u32. * Third reason is that later function called "arc4_crypt" using "const *u8" as parameter. So I think this change is reasonable. >> >> signed int curfragnum, length; >> u32 keylength; >> @@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) >> >> length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >> >> - *((__le32 *)crc) = ~crc32_le(~0, payload, length); >> + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); > >Are you sure this does what you think it does? > >What exactly is this doing now? This change might be wrong totally. I feel sorry for this. arc4_crypt need "const *u8" as parameter, so crc should be type "const *u8". But it use "__le32", so in my opinion, it should be *((u32 *)crc) = ~crc32_le(~0, payload, length); > >> >> arc4_setkey(ctx, wepkey, 3 + keylength); >> arc4_crypt(ctx, payload, payload, length); >> @@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) >> >> } else { >> length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; >> - *((__le32 *)crc) = ~crc32_le(~0, payload, length); >> + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length)); >> arc4_setkey(ctx, wepkey, 3 + keylength); >> arc4_crypt(ctx, payload, payload, length); >> arc4_crypt(ctx, payload + length, crc, 4); >> @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) >> arc4_crypt(ctx, payload, payload, length); >> >> /* calculate icv and compare the icv */ >> - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); >> + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); > >This odd casting feels wrong, why is it correct now? > If we want to keep cpu value, we should fix it like this: *((u32 *)crc) =~crc32_le(~0, payload, length - 4); If we want to keep le32 value, we should fix it like this: *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); These fix are all solve the warnings. The problem is which one should I choose to meet the author's wishes. And in this situation, it is hard to choose which one is better, as after this line, the function return directly. >thanks, > >greg k-h The new fix I choose to meet the author's wishes and fix warnings is following: (Following content is just asked for suggestions and if it were proper, I would send v2.) @@ -36,7 +36,7 @@ const char *security_type_str(u8 value) void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) { /* exclude ICV */ - unsigned char crc[4]; + u8 crc[4]; signed int curfragnum, length; u32 keylength; @@ -69,7 +69,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) =~crc32_le(~0, payload, length); arc4_setkey(ctx, wepkey, 3 + keylength); arc4_crypt(ctx, payload, payload, length); @@ -77,7 +77,7 @@ void rtw_wep_encrypt(struct adapter *padapter, u8 *pxmitframe) } else { length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) = ~crc32_le(~0, payload, length); arc4_setkey(ctx, wepkey, 3 + keylength); arc4_crypt(ctx, payload, payload, length); arc4_crypt(ctx, payload + length, crc, 4); @@ -121,7 +121,7 @@ void rtw_wep_decrypt(struct adapter *padapter, u8 *precvframe) arc4_crypt(ctx, payload, payload, length); /* calculate icv and compare the icv */ - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); *- or *- *- + *((u32 *)crc) = ~crc32_le(~0, payload, length - 4); *- *- // !! This code is confusing, because after this line of code, the function return directly. I don't know what is the author's wish. } } @@ -506,7 +506,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) if ((curfragnum+1) == pattrib->nr_frags) { /* 4 the last fragment */ length = pattrib->last_txcmdsz-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) = ~crc32_le(~0, payload, length); arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); @@ -514,7 +514,7 @@ u32 rtw_tkip_encrypt(struct adapter *padapter, u8 *pxmitframe) } else { length = pxmitpriv->frag_len-pattrib->hdrlen-pattrib->iv_len-pattrib->icv_len; - *((__le32 *)crc) = ~crc32_le(~0, payload, length); + *((u32 *)crc) = ~crc32_le(~0, payload, length); arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); @@ -618,7 +618,7 @@ u32 rtw_tkip_decrypt(struct adapter *padapter, u8 *precvframe) arc4_setkey(ctx, rc4key, 16); arc4_crypt(ctx, payload, payload, length); - *((u32 *)crc) = le32_to_cpu(~crc32_le(~0, payload, length - 4)); + *((u32 *)crc) = ~crc32_le(~0, payload, length - 4); * or * * + *((__le32 *)crc) = cpu_to_le32(~crc32_le(~0, payload, length - 4)); * * //!! This fix also confused me. But, IMO, u32 is better to meet the author's wishes. if (crc[3] != payload[length - 1] || crc[2] != payload[length - 2] || crc[1] != payload[length - 3] || crc[0] != payload[length - 4]) -- 2.7.4 I think new patch is better, ;). Please review. Thanks, Jiabing ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-06-04 2:20 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-06-03 8:24 [PATCH] staging: rtl8723bs: core: fix some incorrect type warnings Wan Jiabing 2021-06-03 8:36 ` Jiabing Wan 2021-06-03 8:36 ` Jiabing Wan 2021-06-03 9:17 ` [PATCH] " Greg Kroah-Hartman 2021-06-03 9:18 ` Greg Kroah-Hartman 2021-06-04 2:19 ` Jiabing Wan 2021-06-04 2:19 ` Jiabing Wan
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.