linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: rtl8712: avoid a useless call to memset().
@ 2012-12-05  1:22 Cyril Roelandt
  2012-12-05  4:07 ` Larry Finger
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Cyril Roelandt @ 2012-12-05  1:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: kernel-janitors, Larry.Finger, florian.c.schilhabel, gregkh,
	yongjun_wei, jim.cromie, marcos.souza.org, devel, Cyril Roelandt

In r8711_wx_get_wap(), make sure we do not call memcpy() on a memory area that
has just been zeroed by a call to memset().

Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_ioctl_linux.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 3a64790..2c73319 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -1131,10 +1131,11 @@ static int r8711_wx_get_wap(struct net_device *dev,
 	struct ndis_wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
 
 	wrqu->ap_addr.sa_family = ARPHRD_ETHER;
-	memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);
 	if (check_fwstate(pmlmepriv, _FW_LINKED |
 	    WIFI_ADHOC_MASTER_STATE|WIFI_AP_STATE)) {
 		memcpy(wrqu->ap_addr.sa_data, pcur_bss->MacAddress, ETH_ALEN);
+	} else {
+		memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);
 	}
 	return 0;
 }
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8712: avoid a useless call to memset().
  2012-12-05  1:22 [PATCH] staging: rtl8712: avoid a useless call to memset() Cyril Roelandt
@ 2012-12-05  4:07 ` Larry Finger
  2012-12-05  7:11 ` Dan Carpenter
  2012-12-05  8:12 ` walter harms
  2 siblings, 0 replies; 6+ messages in thread
From: Larry Finger @ 2012-12-05  4:07 UTC (permalink / raw)
  To: Cyril Roelandt
  Cc: linux-kernel, kernel-janitors, florian.c.schilhabel, gregkh,
	yongjun_wei, jim.cromie, marcos.souza.org, devel

On 12/04/2012 07:22 PM, Cyril Roelandt wrote:
> In r8711_wx_get_wap(), make sure we do not call memcpy() on a memory area that
> has just been zeroed by a call to memset().
>
> Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
> ---
>   drivers/staging/rtl8712/rtl871x_ioctl_linux.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)

Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Thanks,

Larry

>
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 3a64790..2c73319 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -1131,10 +1131,11 @@ static int r8711_wx_get_wap(struct net_device *dev,
>   	struct ndis_wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
>
>   	wrqu->ap_addr.sa_family = ARPHRD_ETHER;
> -	memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);
>   	if (check_fwstate(pmlmepriv, _FW_LINKED |
>   	    WIFI_ADHOC_MASTER_STATE|WIFI_AP_STATE)) {
>   		memcpy(wrqu->ap_addr.sa_data, pcur_bss->MacAddress, ETH_ALEN);
> +	} else {
> +		memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);
>   	}
>   	return 0;
>   }
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8712: avoid a useless call to memset().
  2012-12-05  1:22 [PATCH] staging: rtl8712: avoid a useless call to memset() Cyril Roelandt
  2012-12-05  4:07 ` Larry Finger
@ 2012-12-05  7:11 ` Dan Carpenter
  2012-12-09  0:31   ` Cyril Roelandt
  2012-12-05  8:12 ` walter harms
  2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2012-12-05  7:11 UTC (permalink / raw)
  To: Cyril Roelandt
  Cc: linux-kernel, devel, florian.c.schilhabel, gregkh,
	kernel-janitors, yongjun_wei, marcos.souza.org, Larry.Finger

On Wed, Dec 05, 2012 at 02:22:02AM +0100, Cyril Roelandt wrote:
> In r8711_wx_get_wap(), make sure we do not call memcpy() on a memory area that
> has just been zeroed by a call to memset().
> 

I look at it like the original code is fine.  Your version is also
fine but is it worth the churn?  Also the curly braces are not
needed.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8712: avoid a useless call to memset().
  2012-12-05  1:22 [PATCH] staging: rtl8712: avoid a useless call to memset() Cyril Roelandt
  2012-12-05  4:07 ` Larry Finger
  2012-12-05  7:11 ` Dan Carpenter
@ 2012-12-05  8:12 ` walter harms
  2 siblings, 0 replies; 6+ messages in thread
From: walter harms @ 2012-12-05  8:12 UTC (permalink / raw)
  To: Cyril Roelandt
  Cc: linux-kernel, kernel-janitors, Larry.Finger, florian.c.schilhabel,
	gregkh, yongjun_wei, jim.cromie, marcos.souza.org, devel

Hi Cyril,
you patch is correct ..but being an advocate of defensive programming
i would say the original version especially since the number of bytes is small.

re,
 wh

Am 05.12.2012 02:22, schrieb Cyril Roelandt:
> In r8711_wx_get_wap(), make sure we do not call memcpy() on a memory area that
> has just been zeroed by a call to memset().
> 
> Signed-off-by: Cyril Roelandt <tipecaml@gmail.com>
> ---
>  drivers/staging/rtl8712/rtl871x_ioctl_linux.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> index 3a64790..2c73319 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
> @@ -1131,10 +1131,11 @@ static int r8711_wx_get_wap(struct net_device *dev,
>  	struct ndis_wlan_bssid_ex *pcur_bss = &pmlmepriv->cur_network.network;
>  
>  	wrqu->ap_addr.sa_family = ARPHRD_ETHER;
> -	memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);
>  	if (check_fwstate(pmlmepriv, _FW_LINKED |
>  	    WIFI_ADHOC_MASTER_STATE|WIFI_AP_STATE)) {
>  		memcpy(wrqu->ap_addr.sa_data, pcur_bss->MacAddress, ETH_ALEN);
> +	} else {
> +		memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);
>  	}
>  	return 0;
>  }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8712: avoid a useless call to memset().
  2012-12-05  7:11 ` Dan Carpenter
@ 2012-12-09  0:31   ` Cyril Roelandt
  2012-12-10  8:55     ` Dan Carpenter
  0 siblings, 1 reply; 6+ messages in thread
From: Cyril Roelandt @ 2012-12-09  0:31 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: linux-kernel, devel, florian.c.schilhabel, gregkh,
	kernel-janitors, yongjun_wei, marcos.souza.org, Larry.Finger

On 12/05/2012 08:11 AM, Dan Carpenter wrote:
> On Wed, Dec 05, 2012 at 02:22:02AM +0100, Cyril Roelandt wrote:
>> In r8711_wx_get_wap(), make sure we do not call memcpy() on a memory area that
>> has just been zeroed by a call to memset().
>>
>
> I look at it like the original code is fine.  Your version is also
> fine but is it worth the churn?  Also the curly braces are not
> needed.
>

Sorry about the braces.

I just thought the code would be easier to understand this way, but it's 
probably OK to leave it as it currently is too.

WBR,
Cyril Roelandt.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] staging: rtl8712: avoid a useless call to memset().
  2012-12-09  0:31   ` Cyril Roelandt
@ 2012-12-10  8:55     ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-12-10  8:55 UTC (permalink / raw)
  To: Cyril Roelandt
  Cc: devel, florian.c.schilhabel, gregkh, kernel-janitors,
	linux-kernel, yongjun_wei, marcos.souza.org, Larry.Finger

On Sun, Dec 09, 2012 at 01:31:50AM +0100, Cyril Roelandt wrote:
> On 12/05/2012 08:11 AM, Dan Carpenter wrote:
> >On Wed, Dec 05, 2012 at 02:22:02AM +0100, Cyril Roelandt wrote:
> >>In r8711_wx_get_wap(), make sure we do not call memcpy() on a memory area that
> >>has just been zeroed by a call to memset().
> >>
> >
> >I look at it like the original code is fine.  Your version is also
> >fine but is it worth the churn?  Also the curly braces are not
> >needed.
> >
> 
> Sorry about the braces.
> 
> I just thought the code would be easier to understand this way, but
> it's probably OK to leave it as it currently is too.
> 

Ah.  I'd buy that.  I like readability fixes.  The indenting is also
messed up and confusing.  It should be:

	if (check_fwstate(pmlmepriv, _FW_LINKED | WIFI_ADHOC_MASTER_STATE |
				     WIFI_AP_STATE))
		memcpy(wrqu->ap_addr.sa_data, pcur_bss->MacAddress, ETH_ALEN);
	else
		memset(wrqu->ap_addr.sa_data, 0, ETH_ALEN);

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-12-10  8:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-05  1:22 [PATCH] staging: rtl8712: avoid a useless call to memset() Cyril Roelandt
2012-12-05  4:07 ` Larry Finger
2012-12-05  7:11 ` Dan Carpenter
2012-12-09  0:31   ` Cyril Roelandt
2012-12-10  8:55     ` Dan Carpenter
2012-12-05  8:12 ` walter harms

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).