diff mbox

[OPW,kernel] Staging: slicoss: Replace memcpy with struct assignment

Message ID 20131025144008.GA21150@gmail.com
State New, archived
Headers show

Commit Message

Rashika Oct. 25, 2013, 2:40 p.m. UTC
This patch fixes the following coccinelle warning in slicoss.c-
drivers/staging/slicoss/slicoss.c:1152:3-9: Replace memcpy with struct assignment

Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
---
 drivers/staging/slicoss/slicoss.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

gregkh@linuxfoundation.org Oct. 27, 2013, 2:02 p.m. UTC | #1
On Fri, Oct 25, 2013 at 08:10:09PM +0530, Rashika Kheria wrote:
> This patch fixes the following coccinelle warning in slicoss.c-
> drivers/staging/slicoss/slicoss.c:1152:3-9: Replace memcpy with struct assignment
> 
> Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
> ---
>  drivers/staging/slicoss/slicoss.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> index f37558f..8a0a2dd 100644
> --- a/drivers/staging/slicoss/slicoss.c
> +++ b/drivers/staging/slicoss/slicoss.c
> @@ -1149,7 +1149,7 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
>  				    (newstats->rcv_drops_gb -
>  				     old->rcv_drops_gb);
>  			}
> -			memcpy(old, newstats, sizeof(struct slic_stats));
> +			old = newstats;

I don't like relying on the compiler to do this type of thing, so for
now, I'm not going to take this patch, sorry.

greg k-h
Josh Triplett Nov. 3, 2013, 7:22 p.m. UTC | #2
On Sun, Oct 27, 2013 at 07:02:36AM -0700, Greg KH wrote:
> On Fri, Oct 25, 2013 at 08:10:09PM +0530, Rashika Kheria wrote:
> > This patch fixes the following coccinelle warning in slicoss.c-
> > drivers/staging/slicoss/slicoss.c:1152:3-9: Replace memcpy with struct assignment
> > 
> > Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
> > ---
> >  drivers/staging/slicoss/slicoss.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> > index f37558f..8a0a2dd 100644
> > --- a/drivers/staging/slicoss/slicoss.c
> > +++ b/drivers/staging/slicoss/slicoss.c
> > @@ -1149,7 +1149,7 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
> >  				    (newstats->rcv_drops_gb -
> >  				     old->rcv_drops_gb);
> >  			}
> > -			memcpy(old, newstats, sizeof(struct slic_stats));
> > +			old = newstats;
> 
> I don't like relying on the compiler to do this type of thing, so for
> now, I'm not going to take this patch, sorry.

Why not?  Is there some specific issue you've observed with letting the
compiler do aggregate assignments?  Does it do so incorrectly, or less
efficiently?

- Josh Triplett
gregkh@linuxfoundation.org Nov. 3, 2013, 8:47 p.m. UTC | #3
On Sun, Nov 03, 2013 at 11:22:45AM -0800, Josh Triplett wrote:
> On Sun, Oct 27, 2013 at 07:02:36AM -0700, Greg KH wrote:
> > On Fri, Oct 25, 2013 at 08:10:09PM +0530, Rashika Kheria wrote:
> > > This patch fixes the following coccinelle warning in slicoss.c-
> > > drivers/staging/slicoss/slicoss.c:1152:3-9: Replace memcpy with struct assignment
> > > 
> > > Signed-off-by: Rashika Kheria <rashika.kheria@gmail.com>
> > > ---
> > >  drivers/staging/slicoss/slicoss.c |    2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
> > > index f37558f..8a0a2dd 100644
> > > --- a/drivers/staging/slicoss/slicoss.c
> > > +++ b/drivers/staging/slicoss/slicoss.c
> > > @@ -1149,7 +1149,7 @@ static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
> > >  				    (newstats->rcv_drops_gb -
> > >  				     old->rcv_drops_gb);
> > >  			}
> > > -			memcpy(old, newstats, sizeof(struct slic_stats));
> > > +			old = newstats;
> > 
> > I don't like relying on the compiler to do this type of thing, so for
> > now, I'm not going to take this patch, sorry.
> 
> Why not?  Is there some specific issue you've observed with letting the
> compiler do aggregate assignments?  Does it do so incorrectly, or less
> efficiently?

For the same reason kernel developers eschew typedefs, you are hiding
the fact that this is a structure being assigned/modified and this is
really a memcpy happening.  Both lines work just as fast (in fact, I'd
argue that the memcpy is probably faster due to arch-specific
optimizations that the compiler knows about.)

thanks,

greg k-h
diff mbox

Patch

diff --git a/drivers/staging/slicoss/slicoss.c b/drivers/staging/slicoss/slicoss.c
index f37558f..8a0a2dd 100644
--- a/drivers/staging/slicoss/slicoss.c
+++ b/drivers/staging/slicoss/slicoss.c
@@ -1149,7 +1149,7 @@  static void slic_upr_request_complete(struct adapter *adapter, u32 isr)
 				    (newstats->rcv_drops_gb -
 				     old->rcv_drops_gb);
 			}
-			memcpy(old, newstats, sizeof(struct slic_stats));
+			old = newstats;
 			break;
 		}
 	case SLIC_UPR_RLSR: