diff mbox

[OPW,kernel,08/10] staging: rtl8712: Replace memcpy with struct assignement

Message ID ffe6a7640e00089843723cbc7f27054e6f71899e.1412524266.git.tapaswenipathak@gmail.com
State New, archived
Headers show

Commit Message

Tapasweni Pathak Oct. 5, 2014, 4:05 p.m. UTC
This patch replaces this kind of memcpy() uses as it is
error prone. It is replaced with a struct assignment as it is
typesafe and much easier to read.

This is done by Coccinelle. Coccinelle script used:
// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Tapasweni Pathak <tapaswenipathak@gmail.com>
---
 drivers/staging/rtl8712/rtl871x_mlme.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--
1.7.9.5

Comments

Greg KH Oct. 5, 2014, 4:22 p.m. UTC | #1
On Sun, Oct 05, 2014 at 09:35:12PM +0530, Tapasweni Pathak wrote:
> This patch replaces this kind of memcpy() uses as it is
> error prone. It is replaced with a struct assignment as it is
> typesafe and much easier to read.
> 
> This is done by Coccinelle. Coccinelle script used:
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 
> Signed-off-by: Tapasweni Pathak <tapaswenipathak@gmail.com>
> ---
>  drivers/staging/rtl8712/rtl871x_mlme.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
> index 00f2e0f..b32bfee 100644
> --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> @@ -884,8 +884,7 @@ void r8712_joinbss_event_callback(struct _adapter *adapter, u8 *pbuf)
>  			}
> 
>  			/*s3. update cur_network & indicate connect*/
> -			memcpy(&cur_network->network, &pnetwork->network,
> -				pnetwork->network.Length);
> +			cur_network->network = pnetwork->network;

is ->network.Length really the size of the structure itself?

thanks,

greg k-h
Tapasweni Pathak Oct. 7, 2014, 2:31 a.m. UTC | #2
On Sun, Oct 5, 2014 at 9:52 PM, Greg KH <greg@kroah.com> wrote:

> On Sun, Oct 05, 2014 at 09:35:12PM +0530, Tapasweni Pathak wrote:
> > This patch replaces this kind of memcpy() uses as it is
> > error prone. It is replaced with a struct assignment as it is
> > typesafe and much easier to read.
> >
> > This is done by Coccinelle. Coccinelle script used:
> > // <smpl>
> > @@
> > identifier struct_name;
> > struct struct_name to;
> > struct struct_name from;
> > expression E;
> > @@
> > -memcpy(&(to), &(from), E);
> > +to = from;
> > // </smpl>
> >
> > Signed-off-by: Tapasweni Pathak <tapaswenipathak@gmail.com>
> > ---
> >  drivers/staging/rtl8712/rtl871x_mlme.c |    6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c
> b/drivers/staging/rtl8712/rtl871x_mlme.c
> > index 00f2e0f..b32bfee 100644
> > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
> > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
> > @@ -884,8 +884,7 @@ void r8712_joinbss_event_callback(struct _adapter
> *adapter, u8 *pbuf)
> >                       }
> >
> >                       /*s3. update cur_network & indicate connect*/
> > -                     memcpy(&cur_network->network, &pnetwork->network,
> > -                             pnetwork->network.Length);
> > +                     cur_network->network = pnetwork->network;
>
> is ->network.Length really the size of the structure itself?
>
> thanks,
>
> greg k-h
>

Hi,

I don't think this is going to work, I checked all of them after applying,
somehow missed this. Everything else looked fine.
Apart from build-test can you please suggest some ways to test such stuff?

Thanks,
Tapasweni
Greg Kroah-Hartman Oct. 7, 2014, 2:39 a.m. UTC | #3
On Tue, Oct 07, 2014 at 08:01:10AM +0530, Tapasweni Pathak wrote:
> 
> 
> 
> 
> 
> On Sun, Oct 5, 2014 at 9:52 PM, Greg KH <greg@kroah.com> wrote:
> 
>     On Sun, Oct 05, 2014 at 09:35:12PM +0530, Tapasweni Pathak wrote:
>     > This patch replaces this kind of memcpy() uses as it is
>     > error prone. It is replaced with a struct assignment as it is
>     > typesafe and much easier to read.
>     >
>     > This is done by Coccinelle. Coccinelle script used:
>     > // <smpl>
>     > @@
>     > identifier struct_name;
>     > struct struct_name to;
>     > struct struct_name from;
>     > expression E;
>     > @@
>     > -memcpy(&(to), &(from), E);
>     > +to = from;
>     > // </smpl>
>     >
>     > Signed-off-by: Tapasweni Pathak <tapaswenipathak@gmail.com>
>     > ---
>     >  drivers/staging/rtl8712/rtl871x_mlme.c |    6 ++----
>     >  1 file changed, 2 insertions(+), 4 deletions(-)
>     >
>     > diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/
>     rtl8712/rtl871x_mlme.c
>     > index 00f2e0f..b32bfee 100644
>     > --- a/drivers/staging/rtl8712/rtl871x_mlme.c
>     > +++ b/drivers/staging/rtl8712/rtl871x_mlme.c
>     > @@ -884,8 +884,7 @@ void r8712_joinbss_event_callback(struct _adapter
>     *adapter, u8 *pbuf)
>     >                       }
>     >
>     >                       /*s3. update cur_network & indicate connect*/
>     > -                     memcpy(&cur_network->network, &pnetwork->network,
>     > -                             pnetwork->network.Length);
>     > +                     cur_network->network = pnetwork->network;
> 
>     is ->network.Length really the size of the structure itself?
> 
>     thanks,
> 
>     greg k-h
> 
> 
> Hi,
> 
> I don't think this is going to work, I checked all of them after applying,
> somehow missed this. Everything else looked fine.
> Apart from build-test can you please suggest some ways to test such stuff?

Read the patch manually, like I have to as a reviewer :)

greg k-h
diff mbox

Patch

diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 00f2e0f..b32bfee 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -884,8 +884,7 @@  void r8712_joinbss_event_callback(struct _adapter *adapter, u8 *pbuf)
 			}

 			/*s3. update cur_network & indicate connect*/
-			memcpy(&cur_network->network, &pnetwork->network,
-				pnetwork->network.Length);
+			cur_network->network = pnetwork->network;
 			cur_network->aid = pnetwork->join_res;
 			/*update fw_state will clr _FW_UNDER_LINKING*/
 			switch (pnetwork->network.InfrastructureMode) {
@@ -1626,8 +1625,7 @@  void r8712_init_registrypriv_dev_network(struct _adapter *adapter)
 	u8 *myhwaddr = myid(peepriv);

 	memcpy(pdev_network->MacAddress, myhwaddr, ETH_ALEN);
-	memcpy(&pdev_network->Ssid, &pregistrypriv->ssid,
-		sizeof(struct ndis_802_11_ssid));
+	pdev_network->Ssid = pregistrypriv->ssid;
 	pdev_network->Configuration.Length =
 			 sizeof(struct NDIS_802_11_CONFIGURATION);
 	pdev_network->Configuration.BeaconPeriod = 100;