Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1238711 > unrolled thread

[PATCH] staging: rtl8723au: Fix Sparse errors in rtl8723a_cmd.c

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging: rtl8723au: Fix Sparse errors in rtl8723a_cmd.c Jacob Kiefer <jtk54@cornell.edu> - 2015-10-03 02:40 +0200
    Re: [PATCH] staging: rtl8723au: Fix Sparse errors in rtl8723a_cmd.c Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2015-10-04 10:50 +0200
    Re: [PATCH] staging: rtl8723au: Fix Sparse errors in rtl8723a_cmd.c Jacob Kiefer <jtk54@cornell.edu> - 2015-10-04 21:50 +0200

#1238711 — [PATCH] staging: rtl8723au: Fix Sparse errors in rtl8723a_cmd.c

From: Jacob Kiefer <jtk54@cornell.edu>

This patch fixes the following sparse errors:


  CHECK   drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
...
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:118:25: warning: incorrect type in assignment (different base types)
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:118:25:    expected unsigned int [unsigned] [usertype] <noident>
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:118:25:    got restricted __le32 [usertype] <noident>
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:130:14: warning: incorrect type in assignment (different base types)
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:130:14:    expected unsigned int [unsigned] [usertype] mask
drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:130:14:    got restricted __le32 [usertype] <noident>
  CC [M]  drivers/staging/rtl8723au/hal/rtl8723a_cmd.o

Signed-off-by: Jacob Kiefer <jtk54@cornell.edu>
---
 drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
index 9733aa6..111a24d 100644
--- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
+++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
@@ -115,9 +115,11 @@ exit:
 
 int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
 {
-	*((u32 *)param) = cpu_to_le32(*((u32 *)param));
+	__le32 leparam;
 
-	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);
+	leparam = cpu_to_le32(*((u32 *)param));
+
+	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (u8 *)&leparam);
 
 	return _SUCCESS;
 }
@@ -125,10 +127,11 @@ int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
 int rtl8723a_set_raid_cmd(struct rtw_adapter *padapter, u32 mask, u8 arg)
 {
 	u8 buf[5];
+	__le32 lemask;
 
 	memset(buf, 0, 5);
-	mask = cpu_to_le32(mask);
-	memcpy(buf, &mask, 4);
+	lemask = cpu_to_le32(mask);
+	memcpy(buf, (u32 *)&lemask, 4);
 	buf[4]  = arg;
 
 	FillH2CCmd(padapter, MACID_CONFIG_EID, 5, buf);
-- 
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1239070

On Fri, Oct 02, 2015 at 08:36:28PM -0400, Jacob Kiefer wrote:
> From: Jacob Kiefer <jtk54@cornell.edu>
> 
> This patch fixes the following sparse errors:
> 
> 
>   CHECK   drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> ...
> drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:118:25: warning: incorrect type in assignment (different base types)
> drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:118:25:    expected unsigned int [unsigned] [usertype] <noident>
> drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:118:25:    got restricted __le32 [usertype] <noident>
> drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:130:14: warning: incorrect type in assignment (different base types)
> drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:130:14:    expected unsigned int [unsigned] [usertype] mask
> drivers/staging/rtl8723au/hal/rtl8723a_cmd.c:130:14:    got restricted __le32 [usertype] <noident>
>   CC [M]  drivers/staging/rtl8723au/hal/rtl8723a_cmd.o
> 
> Signed-off-by: Jacob Kiefer <jtk54@cornell.edu>
> ---
>  drivers/staging/rtl8723au/hal/rtl8723a_cmd.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> index 9733aa6..111a24d 100644
> --- a/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> +++ b/drivers/staging/rtl8723au/hal/rtl8723a_cmd.c
> @@ -115,9 +115,11 @@ exit:
>  
>  int rtl8723a_set_rssi_cmd(struct rtw_adapter *padapter, u8 *param)
>  {
> -	*((u32 *)param) = cpu_to_le32(*((u32 *)param));
> +	__le32 leparam;

Why __le32?  Does this variable go across the user/kernel boundry
somehow?  If not, just use le32.

>  
> -	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, param);
> +	leparam = cpu_to_le32(*((u32 *)param));
> +
> +	FillH2CCmd(padapter, RSSI_SETTING_EID, 3, (u8 *)&leparam);

At first glance, you aren't doing ths same logic in this function as the
original did, please look at this very closely again and verify that you
are doing this correctly.

Don't just blindly quiet tools like sparse, it is warning for a reason,
but be careful about your fix.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1239196

Hi Greg,

Thanks for the response! It's always good to get notes on a patch.
Some responses to your points:

> Why __le32?  Does this variable go across the user/kernel boundry
> somehow?  If not, just use le32.

Good point, this should probably have been le32.

> At first glance, you aren't doing ths same logic in this function as the
> original did, please look at this very closely again and verify that you
> are doing this correctly.
>
> Don't just blindly quiet tools like sparse, it is warning for a reason,
> but be careful about your fix.

On a second, closer look at the code I am not doing this correctly: the
buffer I am converting to le32 needs to persist (which a local variable
would not). On my first glance at this code I saw the same buffer being
used for both little- and big-endian storage of the same data -- it's
correct, but a little ugly.

I am going to leave this code as is, since it was functioning properly
before my patch.

Thanks,
Jake
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web