diff mbox

[OPW,kernel] staging: gdm724x: replace memcpy with struct assignment

Message ID 1382562936-29454-1-git-send-email-teobaluta@gmail.com
State New, archived
Headers show

Commit Message

Teodora Baluta Oct. 23, 2013, 9:15 p.m. UTC
This patch fixes the following coccinelle issues:

drivers/staging/gdm724x/gdm_lte.c:153:1-7: Replace memcpy with struct assignment
drivers/staging/gdm724x/gdm_lte.c:302:2-8: Replace memcpy with struct assignment

Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
---
 drivers/staging/gdm724x/gdm_lte.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Rusty Russell Oct. 24, 2013, 2:49 a.m. UTC | #1
Teodora Baluta <teobaluta@gmail.com> writes:
> This patch fixes the following coccinelle issues:
>
> drivers/staging/gdm724x/gdm_lte.c:153:1-7: Replace memcpy with struct assignment
> drivers/staging/gdm724x/gdm_lte.c:302:2-8: Replace memcpy with struct assignment
>
> Signed-off-by: Teodora Baluta <teobaluta@gmail.com>

Reviewed-by: Rusty Russell <rusty@rustcorp.com.au>

Cheers,
Rusty.

> ---
>  drivers/staging/gdm724x/gdm_lte.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/gdm724x/gdm_lte.c b/drivers/staging/gdm724x/gdm_lte.c
> index c57a6ba..c1f68d1 100644
> --- a/drivers/staging/gdm724x/gdm_lte.c
> +++ b/drivers/staging/gdm724x/gdm_lte.c
> @@ -150,7 +150,7 @@ static int gdm_lte_emulate_arp(struct sk_buff *skb_in, u32 nic_type)
>  	arp_data_out = (struct arpdata *)(arp_temp + sizeof(struct arphdr));
>  
>  	/* Copy the arp header */
> -	memcpy(arp_out, arp_in, sizeof(struct arphdr));
> +	*arp_out = *arp_in;
>  	arp_out->ar_op = htons(ARPOP_REPLY);
>  
>  	/* Copy the arp payload: based on 2 bytes of mac and fill the IP */
> @@ -299,7 +299,7 @@ static int gdm_lte_emulate_ndp(struct sk_buff *skb_in, u32 nic_type)
>  		na.link_layer_address[4] = 0x63;
>  		na.link_layer_address[5] = 0xc7;
>  
> -		memcpy(&ipv6_out, ipv6_in, sizeof(struct ipv6hdr));
> +		ipv6_out = *ipv6_in;
>  		memcpy(ipv6_out.saddr.in6_u.u6_addr8, &na.target_address, 16);
>  		memcpy(ipv6_out.daddr.in6_u.u6_addr8, ipv6_in->saddr.in6_u.u6_addr8, 16);
>  		ipv6_out.payload_len = htons(sizeof(struct icmp6hdr) + sizeof(struct neighbour_advertisement));
> -- 
> 1.7.10.4
>
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
gregkh@linuxfoundation.org Oct. 28, 2013, 3:31 a.m. UTC | #2
On Thu, Oct 24, 2013 at 12:15:36AM +0300, Teodora Baluta wrote:
> This patch fixes the following coccinelle issues:
> 
> drivers/staging/gdm724x/gdm_lte.c:153:1-7: Replace memcpy with struct assignment
> drivers/staging/gdm724x/gdm_lte.c:302:2-8: Replace memcpy with struct assignment
> 
> Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
> ---
>  drivers/staging/gdm724x/gdm_lte.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/gdm724x/gdm_lte.c b/drivers/staging/gdm724x/gdm_lte.c
> index c57a6ba..c1f68d1 100644
> --- a/drivers/staging/gdm724x/gdm_lte.c
> +++ b/drivers/staging/gdm724x/gdm_lte.c
> @@ -150,7 +150,7 @@ static int gdm_lte_emulate_arp(struct sk_buff *skb_in, u32 nic_type)
>  	arp_data_out = (struct arpdata *)(arp_temp + sizeof(struct arphdr));
>  
>  	/* Copy the arp header */
> -	memcpy(arp_out, arp_in, sizeof(struct arphdr));
> +	*arp_out = *arp_in;

I really am not comfortable with this type of change, sorry.

greg k-h
Josh Triplett Nov. 3, 2013, 7:06 p.m. UTC | #3
On Sun, Oct 27, 2013 at 08:31:26PM -0700, Greg KH wrote:
> On Thu, Oct 24, 2013 at 12:15:36AM +0300, Teodora Baluta wrote:
> > This patch fixes the following coccinelle issues:
> > 
> > drivers/staging/gdm724x/gdm_lte.c:153:1-7: Replace memcpy with struct assignment
> > drivers/staging/gdm724x/gdm_lte.c:302:2-8: Replace memcpy with struct assignment
> > 
> > Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
> > ---
> >  drivers/staging/gdm724x/gdm_lte.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/gdm724x/gdm_lte.c b/drivers/staging/gdm724x/gdm_lte.c
> > index c57a6ba..c1f68d1 100644
> > --- a/drivers/staging/gdm724x/gdm_lte.c
> > +++ b/drivers/staging/gdm724x/gdm_lte.c
> > @@ -150,7 +150,7 @@ static int gdm_lte_emulate_arp(struct sk_buff *skb_in, u32 nic_type)
> >  	arp_data_out = (struct arpdata *)(arp_temp + sizeof(struct arphdr));
> >  
> >  	/* Copy the arp header */
> > -	memcpy(arp_out, arp_in, sizeof(struct arphdr));
> > +	*arp_out = *arp_in;
> 
> I really am not comfortable with this type of change, sorry.

Can you elaborate on that?

- Josh Triplett
gregkh@linuxfoundation.org Nov. 3, 2013, 8:48 p.m. UTC | #4
On Sun, Nov 03, 2013 at 11:06:19AM -0800, Josh Triplett wrote:
> On Sun, Oct 27, 2013 at 08:31:26PM -0700, Greg KH wrote:
> > On Thu, Oct 24, 2013 at 12:15:36AM +0300, Teodora Baluta wrote:
> > > This patch fixes the following coccinelle issues:
> > > 
> > > drivers/staging/gdm724x/gdm_lte.c:153:1-7: Replace memcpy with struct assignment
> > > drivers/staging/gdm724x/gdm_lte.c:302:2-8: Replace memcpy with struct assignment
> > > 
> > > Signed-off-by: Teodora Baluta <teobaluta@gmail.com>
> > > ---
> > >  drivers/staging/gdm724x/gdm_lte.c |    4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/gdm724x/gdm_lte.c b/drivers/staging/gdm724x/gdm_lte.c
> > > index c57a6ba..c1f68d1 100644
> > > --- a/drivers/staging/gdm724x/gdm_lte.c
> > > +++ b/drivers/staging/gdm724x/gdm_lte.c
> > > @@ -150,7 +150,7 @@ static int gdm_lte_emulate_arp(struct sk_buff *skb_in, u32 nic_type)
> > >  	arp_data_out = (struct arpdata *)(arp_temp + sizeof(struct arphdr));
> > >  
> > >  	/* Copy the arp header */
> > > -	memcpy(arp_out, arp_in, sizeof(struct arphdr));
> > > +	*arp_out = *arp_in;
> > 
> > I really am not comfortable with this type of change, sorry.
> 
> Can you elaborate on that?

See my other comment about why you should be doing a memcpy instead of
"hiding it".

thanks,

greg k-h
diff mbox

Patch

diff --git a/drivers/staging/gdm724x/gdm_lte.c b/drivers/staging/gdm724x/gdm_lte.c
index c57a6ba..c1f68d1 100644
--- a/drivers/staging/gdm724x/gdm_lte.c
+++ b/drivers/staging/gdm724x/gdm_lte.c
@@ -150,7 +150,7 @@  static int gdm_lte_emulate_arp(struct sk_buff *skb_in, u32 nic_type)
 	arp_data_out = (struct arpdata *)(arp_temp + sizeof(struct arphdr));
 
 	/* Copy the arp header */
-	memcpy(arp_out, arp_in, sizeof(struct arphdr));
+	*arp_out = *arp_in;
 	arp_out->ar_op = htons(ARPOP_REPLY);
 
 	/* Copy the arp payload: based on 2 bytes of mac and fill the IP */
@@ -299,7 +299,7 @@  static int gdm_lte_emulate_ndp(struct sk_buff *skb_in, u32 nic_type)
 		na.link_layer_address[4] = 0x63;
 		na.link_layer_address[5] = 0xc7;
 
-		memcpy(&ipv6_out, ipv6_in, sizeof(struct ipv6hdr));
+		ipv6_out = *ipv6_in;
 		memcpy(ipv6_out.saddr.in6_u.u6_addr8, &na.target_address, 16);
 		memcpy(ipv6_out.daddr.in6_u.u6_addr8, ipv6_in->saddr.in6_u.u6_addr8, 16);
 		ipv6_out.payload_len = htons(sizeof(struct icmp6hdr) + sizeof(struct neighbour_advertisement));