[PATCH 01/23] uvc: Replace memcpy with struct assignment

[PATCH 01/23] uvc: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index f00db30..4fc8737 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming *stream,
 		goto done;
 	}
 
-	memcpy(&stream->ctrl, &probe, sizeof probe);
+	stream->ctrl = probe;
 	stream->cur_format = format;
 	stream->cur_frame = frame;
 
@@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
 		return -EBUSY;
 	}
 
-	memcpy(&probe, &stream->ctrl, sizeof probe);
+	probe = stream->ctrl;
 	probe.dwFrameInterval =
 		uvc_try_frame_interval(stream->cur_frame, interval);
 
@@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
 		return ret;
 	}
 
-	memcpy(&stream->ctrl, &probe, sizeof probe);
+	stream->ctrl = probe;
 	mutex_unlock(&stream->mutex);
 
 	/* Return the actual frame period. */
-- 
1.7.4.4

[PATCH 02/23] cx231xx: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-cards.c |    2 +-
 drivers/media/usb/cx231xx/cx231xx-video.c |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index b84ebc5..352d676 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -705,7 +705,7 @@ void cx231xx_sleep_s5h1432(struct cx231xx *dev)
 
 static inline void cx231xx_set_model(struct cx231xx *dev)
 {
-	memcpy(&dev->board, &cx231xx_boards[dev->model], sizeof(dev->board));
+	dev->board = cx231xx_boards[dev->model];
 }
 
 /* Since cx231xx_pre_card_setup() requires a proper dev->model,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index fedf785..c5109ba 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -2627,8 +2627,7 @@ int cx231xx_register_analog_devices(struct cx231xx *dev)
 		     dev->name, video_device_node_name(dev->vdev));
 
 	/* Initialize VBI template */
-	memcpy(&cx231xx_vbi_template, &cx231xx_video_template,
-	       sizeof(cx231xx_vbi_template));
+	cx231xx_vbi_template = cx231xx_video_template;
 	strcpy(cx231xx_vbi_template.name, "cx231xx-vbi");
 
 	/* Allocate and fill vbi video_device struct */
-- 
1.7.4.4

[PATCH 03/23] usbvision: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/usbvision/usbvision-i2c.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-i2c.c b/drivers/media/usb/usbvision/usbvision-i2c.c
index 89fec02..ba262a3 100644
--- a/drivers/media/usb/usbvision/usbvision-i2c.c
+++ b/drivers/media/usb/usbvision/usbvision-i2c.c
@@ -189,8 +189,7 @@ int usbvision_i2c_register(struct usb_usbvision *usbvision)
 	if (usbvision->registered_i2c)
 		return 0;
 
-	memcpy(&usbvision->i2c_adap, &i2c_adap_template,
-	       sizeof(struct i2c_adapter));
+	usbvision->i2c_adap = i2c_adap_template;
 
 	sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
 		usbvision->dev->bus->busnum, usbvision->dev->devpath);
-- 
1.7.4.4

[PATCH 04/23] sn9c102: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/sn9c102/sn9c102_core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/sn9c102/sn9c102_core.c b/drivers/media/usb/sn9c102/sn9c102_core.c
index 5bfc8e2..4cae6f8 100644
--- a/drivers/media/usb/sn9c102/sn9c102_core.c
+++ b/drivers/media/usb/sn9c102/sn9c102_core.c
@@ -2824,7 +2824,7 @@ sn9c102_vidioc_querybuf(struct sn9c102_device* cam, void __user * arg)
 	    b.index >= cam->nbuffers || cam->io != IO_MMAP)
 		return -EINVAL;
 
-	memcpy(&b, &cam->frame[b.index].buf, sizeof(b));
+	b = cam->frame[b.index].buf;
 
 	if (cam->frame[b.index].vma_use_count)
 		b.flags |= V4L2_BUF_FLAG_MAPPED;
@@ -2927,7 +2927,7 @@ sn9c102_vidioc_dqbuf(struct sn9c102_device* cam, struct file* filp,
 
 	f->state = F_UNUSED;
 
-	memcpy(&b, &f->buf, sizeof(b));
+	b = f->buf;
 	if (f->vma_use_count)
 		b.flags |= V4L2_BUF_FLAG_MAPPED;
 
-- 
1.7.4.4

Re: [PATCH 04/23] sn9c102: Replace memcpy with struct assignment

[Ezequiel Garcia]

On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
>
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/usb/sn9c102/sn9c102_core.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/usb/sn9c102/sn9c102_core.c b/drivers/media/usb/sn9c102/sn9c102_core.c
> index 5bfc8e2..4cae6f8 100644
> --- a/drivers/media/usb/sn9c102/sn9c102_core.c
> +++ b/drivers/media/usb/sn9c102/sn9c102_core.c
> @@ -2824,7 +2824,7 @@ sn9c102_vidioc_querybuf(struct sn9c102_device* cam, void __user * arg)
>             b.index >= cam->nbuffers || cam->io != IO_MMAP)
>                 return -EINVAL;
>
> -       memcpy(&b, &cam->frame[b.index].buf, sizeof(b));
> +       b = cam->frame[b.index].buf;
>
>         if (cam->frame[b.index].vma_use_count)
>                 b.flags |= V4L2_BUF_FLAG_MAPPED;
> @@ -2927,7 +2927,7 @@ sn9c102_vidioc_dqbuf(struct sn9c102_device* cam, struct file* filp,
>
>         f->state = F_UNUSED;
>
> -       memcpy(&b, &f->buf, sizeof(b));
> +       b = f->buf;
>         if (f->vma_use_count)
>                 b.flags |= V4L2_BUF_FLAG_MAPPED;
>

Andy: you got me thinking on performance.
Most patches are initialization or setup code.

Here we patch a xxx_vidioc_dqbuf() function.
Is this a speed sensitive path?

I still think this change can't hurt performance,
but I may be wrong!


    Ezequiel

[PATCH 05/23] pwc: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/pwc/pwc-if.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 42e36ba..cc5c7d8 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -1003,7 +1003,7 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
 	vb2_queue_init(&pdev->vb_queue);
 
 	/* Init video_device structure */
-	memcpy(&pdev->vdev, &pwc_template, sizeof(pwc_template));
+	pdev->vdev = pwc_template;
 	strcpy(pdev->vdev.name, name);
 	pdev->vdev.queue = &pdev->vb_queue;
 	pdev->vdev.queue->lock = &pdev->vb_queue_lock;
-- 
1.7.4.4

[PATCH 06/23] pvrusb2: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Cc: Mike Isely <isely@pobox.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/pvrusb2/pvrusb2-encoder.c  |    3 +--
 drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c |    4 ++--
 drivers/media/usb/pvrusb2/pvrusb2-v4l2.c     |    2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/pvrusb2/pvrusb2-encoder.c b/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
index e046fda..f7702ae 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-encoder.c
@@ -422,8 +422,7 @@ int pvr2_encoder_adjust(struct pvr2_hdw *hdw)
 		pvr2_trace(PVR2_TRACE_ERROR_LEGS,
 			   "Error from cx2341x module code=%d",ret);
 	} else {
-		memcpy(&hdw->enc_cur_state,&hdw->enc_ctl_state,
-		       sizeof(struct cx2341x_mpeg_params));
+		hdw->enc_cur_state = hdw->enc_ctl_state;
 		hdw->enc_cur_valid = !0;
 	}
 	return ret;
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c b/drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c
index 885ce11..9691156 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-i2c-core.c
@@ -649,8 +649,8 @@ void pvr2_i2c_core_init(struct pvr2_hdw *hdw)
 	}
 
 	// Configure the adapter and set up everything else related to it.
-	memcpy(&hdw->i2c_adap,&pvr2_i2c_adap_template,sizeof(hdw->i2c_adap));
-	memcpy(&hdw->i2c_algo,&pvr2_i2c_algo_template,sizeof(hdw->i2c_algo));
+	hdw->i2c_adap = pvr2_i2c_adap_template;
+	hdw->i2c_algo = pvr2_i2c_algo_template;
 	strlcpy(hdw->i2c_adap.name,hdw->name,sizeof(hdw->i2c_adap.name));
 	hdw->i2c_adap.dev.parent = &hdw->usb_dev->dev;
 	hdw->i2c_adap.algo = &hdw->i2c_algo;
diff --git a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
index db249ca..5b622ec 100644
--- a/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
+++ b/drivers/media/usb/pvrusb2/pvrusb2-v4l2.c
@@ -1339,7 +1339,7 @@ static void pvr2_v4l2_dev_init(struct pvr2_v4l2_dev *dip,
 		return;
 	}
 
-	memcpy(&dip->devbase,&vdev_template,sizeof(vdev_template));
+	dip->devbase = vdev_template;
 	dip->devbase.release = pvr2_video_device_release;
 	dip->devbase.ioctl_ops = &pvr2_ioctl_ops;
 	{
-- 
1.7.4.4

[PATCH 07/23] hdpvr: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Cc: Mike Isely <isely@pobox.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/hdpvr/hdpvr-i2c.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/hdpvr/hdpvr-i2c.c b/drivers/media/usb/hdpvr/hdpvr-i2c.c
index 82e819f..2df60bf 100644
--- a/drivers/media/usb/hdpvr/hdpvr-i2c.c
+++ b/drivers/media/usb/hdpvr/hdpvr-i2c.c
@@ -217,8 +217,7 @@ int hdpvr_register_i2c_adapter(struct hdpvr_device *dev)
 
 	hdpvr_activate_ir(dev);
 
-	memcpy(&dev->i2c_adapter, &hdpvr_i2c_adapter_template,
-	       sizeof(struct i2c_adapter));
+	dev->i2c_adapter = hdpvr_i2c_adapter_template;
 	dev->i2c_adapter.dev.parent = &dev->udev->dev;
 
 	i2c_set_adapdata(&dev->i2c_adapter, dev);
-- 
1.7.4.4

[PATCH 08/23] cx25840: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/i2c/cx25840/cx25840-ir.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/i2c/cx25840/cx25840-ir.c b/drivers/media/i2c/cx25840/cx25840-ir.c
index 38ce76e..9ae977b 100644
--- a/drivers/media/i2c/cx25840/cx25840-ir.c
+++ b/drivers/media/i2c/cx25840/cx25840-ir.c
@@ -1251,13 +1251,11 @@ int cx25840_ir_probe(struct v4l2_subdev *sd)
 		cx25840_write4(ir_state->c, CX25840_IR_IRQEN_REG, 0);
 
 	mutex_init(&ir_state->rx_params_lock);
-	memcpy(&default_params, &default_rx_params,
-		       sizeof(struct v4l2_subdev_ir_parameters));
+	default_params = default_rx_params;
 	v4l2_subdev_call(sd, ir, rx_s_parameters, &default_params);
 
 	mutex_init(&ir_state->tx_params_lock);
-	memcpy(&default_params, &default_tx_params,
-		       sizeof(struct v4l2_subdev_ir_parameters));
+	default_params = default_tx_params;
 	v4l2_subdev_call(sd, ir, tx_s_parameters, &default_params);
 
 	return 0;
-- 
1.7.4.4

Re: [PATCH 08/23] cx25840: Replace memcpy with struct assignment

[Andy Walls]

On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
> 
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 

This patch is fine.

Signed-off-by: Andy Walls <awalls@md.metrocast.net>

> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/i2c/cx25840/cx25840-ir.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/i2c/cx25840/cx25840-ir.c b/drivers/media/i2c/cx25840/cx25840-ir.c
> index 38ce76e..9ae977b 100644
> --- a/drivers/media/i2c/cx25840/cx25840-ir.c
> +++ b/drivers/media/i2c/cx25840/cx25840-ir.c
> @@ -1251,13 +1251,11 @@ int cx25840_ir_probe(struct v4l2_subdev *sd)
>  		cx25840_write4(ir_state->c, CX25840_IR_IRQEN_REG, 0);
>  
>  	mutex_init(&ir_state->rx_params_lock);
> -	memcpy(&default_params, &default_rx_params,
> -		       sizeof(struct v4l2_subdev_ir_parameters));
> +	default_params = default_rx_params;
>  	v4l2_subdev_call(sd, ir, rx_s_parameters, &default_params);
>  
>  	mutex_init(&ir_state->tx_params_lock);
> -	memcpy(&default_params, &default_tx_params,
> -		       sizeof(struct v4l2_subdev_ir_parameters));
> +	default_params = default_tx_params;
>  	v4l2_subdev_call(sd, ir, tx_s_parameters, &default_params);
>  
>  	return 0;

[PATCH 09/23] zr36067: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/pci/zoran/zoran_card.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/zoran/zoran_card.c b/drivers/media/pci/zoran/zoran_card.c
index fffc54b..cea325d 100644
--- a/drivers/media/pci/zoran/zoran_card.c
+++ b/drivers/media/pci/zoran/zoran_card.c
@@ -708,8 +708,7 @@ static const struct i2c_algo_bit_data zoran_i2c_bit_data_template = {
 static int
 zoran_register_i2c (struct zoran *zr)
 {
-	memcpy(&zr->i2c_algo, &zoran_i2c_bit_data_template,
-	       sizeof(struct i2c_algo_bit_data));
+	zr->i2c_algo = zoran_i2c_bit_data_template;
 	zr->i2c_algo.data = zr;
 	strlcpy(zr->i2c_adapter.name, ZR_DEVNAME(zr),
 		sizeof(zr->i2c_adapter.name));
-- 
1.7.4.4

[PATCH 10/23] dvb-usb/friio-fe: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/dvb-usb/friio-fe.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/media/usb/dvb-usb/friio-fe.c b/drivers/media/usb/dvb-usb/friio-fe.c
index 90a70c6..d56f927 100644
--- a/drivers/media/usb/dvb-usb/friio-fe.c
+++ b/drivers/media/usb/dvb-usb/friio-fe.c
@@ -421,11 +421,10 @@ struct dvb_frontend *jdvbt90502_attach(struct dvb_usb_device *d)
 
 	/* setup the state */
 	state->i2c = &d->i2c_adap;
-	memcpy(&state->config, &friio_fe_config, sizeof(friio_fe_config));
+	state->config = friio_fe_config;
 
 	/* create dvb_frontend */
-	memcpy(&state->frontend.ops, &jdvbt90502_ops,
-	       sizeof(jdvbt90502_ops));
+	state->frontend.ops = jdvbt90502_ops;
 	state->frontend.demodulator_priv = state;
 
 	if (jdvbt90502_init(&state->frontend) < 0)
-- 
1.7.4.4

[PATCH 11/23] au0828: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/usb/au0828/au0828-cards.c |    2 +-
 drivers/media/usb/au0828/au0828-i2c.c   |    9 +++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-cards.c b/drivers/media/usb/au0828/au0828-cards.c
index 448361c..8830d6d 100644
--- a/drivers/media/usb/au0828/au0828-cards.c
+++ b/drivers/media/usb/au0828/au0828-cards.c
@@ -192,7 +192,7 @@ void au0828_card_setup(struct au0828_dev *dev)
 
 	dprintk(1, "%s()\n", __func__);
 
-	memcpy(&dev->board, &au0828_boards[dev->boardnr], sizeof(dev->board));
+	dev->board = au0828_boards[dev->boardnr];
 
 	if (dev->i2c_rc == 0) {
 		dev->i2c_client.addr = 0xa0 >> 1;
diff --git a/drivers/media/usb/au0828/au0828-i2c.c b/drivers/media/usb/au0828/au0828-i2c.c
index 4ded17f..71fbe59 100644
--- a/drivers/media/usb/au0828/au0828-i2c.c
+++ b/drivers/media/usb/au0828/au0828-i2c.c
@@ -364,12 +364,9 @@ int au0828_i2c_register(struct au0828_dev *dev)
 {
 	dprintk(1, "%s()\n", __func__);
 
-	memcpy(&dev->i2c_adap, &au0828_i2c_adap_template,
-	       sizeof(dev->i2c_adap));
-	memcpy(&dev->i2c_algo, &au0828_i2c_algo_template,
-	       sizeof(dev->i2c_algo));
-	memcpy(&dev->i2c_client, &au0828_i2c_client_template,
-	       sizeof(dev->i2c_client));
+	dev->i2c_adap = au0828_i2c_adap_template;
+	dev->i2c_algo = au0828_i2c_algo_template;
+	dev->i2c_client = au0828_i2c_client_template;
 
 	dev->i2c_adap.dev.parent = &dev->usbdev->dev;
 
-- 
1.7.4.4

[PATCH 12/23] tuners/xc4000: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/tuners/xc4000.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/tuners/xc4000.c b/drivers/media/tuners/xc4000.c
index 4937712..d178dee 100644
--- a/drivers/media/tuners/xc4000.c
+++ b/drivers/media/tuners/xc4000.c
@@ -1066,7 +1066,7 @@ check_device:
 		goto fail;
 	}
 
-	memcpy(&priv->cur_fw, &new_fw, sizeof(priv->cur_fw));
+	priv->cur_fw = new_fw;
 
 	/*
 	 * By setting BASE in cur_fw.type only after successfully loading all
-- 
1.7.4.4

[PATCH 13/23] tuners/xc2028: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/tuners/tuner-xc2028.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c
index 7bcb6b0..0945173 100644
--- a/drivers/media/tuners/tuner-xc2028.c
+++ b/drivers/media/tuners/tuner-xc2028.c
@@ -870,7 +870,7 @@ check_device:
 	}
 
 read_not_reliable:
-	memcpy(&priv->cur_fw, &new_fw, sizeof(priv->cur_fw));
+	priv->cur_fw = new_fw;
 
 	/*
 	 * By setting BASE in cur_fw.type only after successfully loading all
-- 
1.7.4.4

Re: [PATCH 13/23] tuners/xc2028: Replace memcpy with struct assignment

[Andy Walls]

On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
> 
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/tuners/tuner-xc2028.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/media/tuners/tuner-xc2028.c b/drivers/media/tuners/tuner-xc2028.c
> index 7bcb6b0..0945173 100644
> --- a/drivers/media/tuners/tuner-xc2028.c
> +++ b/drivers/media/tuners/tuner-xc2028.c
> @@ -870,7 +870,7 @@ check_device:
>  	}
>  
>  read_not_reliable:
> -	memcpy(&priv->cur_fw, &new_fw, sizeof(priv->cur_fw));
> +	priv->cur_fw = new_fw;

This memcpy() can get called almost every time
tuner-xc2028.c:generic_set_freq() is called.  

This copies a 32 byte (I think) structure on a 32 bit machine.

I am assuming the difference in performance between the memcpy and
assignment operator is negligible.

Regards,
Andy


>  	/*
>  	 * By setting BASE in cur_fw.type only after successfully loading all

[PATCH 14/23] tuners/tda18271: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/tuners/tda18271-maps.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/tuners/tda18271-maps.c b/drivers/media/tuners/tda18271-maps.c
index fb881c6..b62e925 100644
--- a/drivers/media/tuners/tda18271-maps.c
+++ b/drivers/media/tuners/tda18271-maps.c
@@ -1290,13 +1290,11 @@ int tda18271_assign_map_layout(struct dvb_frontend *fe)
 	switch (priv->id) {
 	case TDA18271HDC1:
 		priv->maps = &tda18271c1_map_layout;
-		memcpy(&priv->std, &tda18271c1_std_map,
-		       sizeof(struct tda18271_std_map));
+		priv->std = tda18271c1_std_map;
 		break;
 	case TDA18271HDC2:
 		priv->maps = &tda18271c2_map_layout;
-		memcpy(&priv->std, &tda18271c2_std_map,
-		       sizeof(struct tda18271_std_map));
+		priv->std = tda18271c2_std_map;
 		break;
 	default:
 		ret = -EINVAL;
-- 
1.7.4.4

[PATCH 15/23] ivtv: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Cc: Andy Walls <awalls@md.metrocast.net>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/pci/ivtv/ivtv-i2c.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c b/drivers/media/pci/ivtv/ivtv-i2c.c
index d47f41a..27a8466 100644
--- a/drivers/media/pci/ivtv/ivtv-i2c.c
+++ b/drivers/media/pci/ivtv/ivtv-i2c.c
@@ -719,13 +719,10 @@ int init_ivtv_i2c(struct ivtv *itv)
 		return -ENODEV;
 	}
 	if (itv->options.newi2c > 0) {
-		memcpy(&itv->i2c_adap, &ivtv_i2c_adap_hw_template,
-		       sizeof(struct i2c_adapter));
+		itv->i2c_adap = ivtv_i2c_adap_hw_template;
 	} else {
-		memcpy(&itv->i2c_adap, &ivtv_i2c_adap_template,
-		       sizeof(struct i2c_adapter));
-		memcpy(&itv->i2c_algo, &ivtv_i2c_algo_template,
-		       sizeof(struct i2c_algo_bit_data));
+		itv->i2c_adap = ivtv_i2c_adap_template;
+		itv->i2c_algo = ivtv_i2c_algo_template;
 	}
 	itv->i2c_algo.udelay = itv->options.i2c_clock_period / 2;
 	itv->i2c_algo.data = itv;
@@ -735,8 +732,7 @@ int init_ivtv_i2c(struct ivtv *itv)
 		itv->instance);
 	i2c_set_adapdata(&itv->i2c_adap, &itv->v4l2_dev);
 
-	memcpy(&itv->i2c_client, &ivtv_i2c_client_template,
-	       sizeof(struct i2c_client));
+	itv->i2c_client = ivtv_i2c_client_template;
 	itv->i2c_client.adapter = &itv->i2c_adap;
 	itv->i2c_adap.dev.parent = &itv->pdev->dev;
 
-- 
1.7.4.4

Re: [PATCH 15/23] ivtv: Replace memcpy with struct assignment

[Andy Walls]

On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.

This one is a code maintenance win. :)

See my comments at the end for the difference in assembled code on an
AMD x86_64 CPU using
$ gcc --version
gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2)


> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 
> Cc: Andy Walls <awalls@md.metrocast.net>

Signed-off-by: Andy Walls <awalls@md.metrocast.net>


> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/pci/ivtv/ivtv-i2c.c |   12 ++++--------
>  1 files changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c b/drivers/media/pci/ivtv/ivtv-i2c.c
> index d47f41a..27a8466 100644
> --- a/drivers/media/pci/ivtv/ivtv-i2c.c
> +++ b/drivers/media/pci/ivtv/ivtv-i2c.c
> @@ -719,13 +719,10 @@ int init_ivtv_i2c(struct ivtv *itv)
>  		return -ENODEV;
>  	}
>  	if (itv->options.newi2c > 0) {
> -		memcpy(&itv->i2c_adap, &ivtv_i2c_adap_hw_template,
> -		       sizeof(struct i2c_adapter));
> +		itv->i2c_adap = ivtv_i2c_adap_hw_template;
>  	} else {
> -		memcpy(&itv->i2c_adap, &ivtv_i2c_adap_template,
> -		       sizeof(struct i2c_adapter));
> -		memcpy(&itv->i2c_algo, &ivtv_i2c_algo_template,
> -		       sizeof(struct i2c_algo_bit_data));
> +		itv->i2c_adap = ivtv_i2c_adap_template;
> +		itv->i2c_algo = ivtv_i2c_algo_template;
>  	}
>  	itv->i2c_algo.udelay = itv->options.i2c_clock_period / 2;
>  	itv->i2c_algo.data = itv;
> @@ -735,8 +732,7 @@ int init_ivtv_i2c(struct ivtv *itv)
>  		itv->instance);
>  	i2c_set_adapdata(&itv->i2c_adap, &itv->v4l2_dev);
>  
> -	memcpy(&itv->i2c_client, &ivtv_i2c_client_template,
> -	       sizeof(struct i2c_client));
> +	itv->i2c_client = ivtv_i2c_client_template;
>  	itv->i2c_client.adapter = &itv->i2c_adap;
>  	itv->i2c_adap.dev.parent = &itv->pdev->dev;
>  

I looked at the generated assembly with only this last change
implemented:

$ objdump -h -r -d -l -s orig-ivtv-i2c.o.sav | less
[...]
 07e0 00000000 69767476 20696e74 65726e61  ....ivtv interna
 07f0 6c000000 00000000 00000000 00000000  l...............
 0800 00000000 00000000 00000000 00000000  ................
 0810 00000000 00000000 00000000 00000000  ................
 0820 00000000 00000000 00000000 00000000  ................
 0830 00000000 00000000 00000000 00000000  ................
[...]
init_ivtv_i2c():
/home/andy/cx18dev/git/media_tree/drivers/media/video/ivtv/ivtv-i2c.c:738
    13bb:       48 c7 c6 00 00 00 00    mov    $0x0,%rsi
                        13be: R_X86_64_32S      .rodata+0x7e0
    13c2:       48 8d bb 30 04 01 00    lea    0x10430(%rbx),%rdi
    13c9:       b9 5a 00 00 00          mov    $0x5a,%ecx
    13ce:       f3 48 a5                rep movsq %ds:(%rsi),%es:(%rdi)


$ objdump -h -r -d -l -s orig-ivtv-i2c.o.sav | less
[...]
 07e0 00000000 69767476 20696e74 65726e61  ....ivtv interna
 07f0 6c000000 00000000 00000000 00000000  l...............
 0800 00000000 00000000 00000000 00000000  ................
 0810 00000000 00000000 00000000 00000000  ................
 0820 00000000 00000000 00000000 00000000  ................
 0830 00000000 00000000 00000000 00000000  ................
[...]
init_ivtv_i2c():
/home/andy/cx18dev/git/media_tree/drivers/media/video/ivtv/ivtv-i2c.c:738
    13bb:       48 8d bb 30 04 01 00    lea    0x10430(%rbx),%rdi
    13c2:       48 c7 c6 00 00 00 00    mov    $0x0,%rsi
                        13c5: R_X86_64_32S      .rodata+0x7e0
    13c9:       b9 5a 00 00 00          mov    $0x5a,%ecx
    13ce:       f3 48 a5                rep movsq %ds:(%rsi),%es:(%rdi)


The generated code is reordered, but essentially identical.  So I guess
in this instance, the preprocessor defines resolved such that an x86-64
optimized memcpy() function was not used from the linux kernel source.

Since all of these memcpy()'s are only called once for each board at
board initialization, performance here really doesn't matter here
anyway.  (Unless one is insanely trying to shave microseconds off boot
time :P )

With other memcpy()/assignement_operator replacement patches, you may
wish to keep performance in mind, if you are patching a frequently
called function.

Regards,
Andy

Re: [PATCH 15/23] ivtv: Replace memcpy with struct assignment

[Ezequiel Garcia]

Hey Andy,

On Tue, Oct 23, 2012 at 7:08 PM, Andy Walls <awalls@md.metrocast.net> wrote:
> On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
>> This kind of memcpy() is error-prone. Its replacement with a struct
>> assignment is prefered because it's type-safe and much easier to read.
>
> This one is a code maintenance win. :)
>
> See my comments at the end for the difference in assembled code on an
> AMD x86_64 CPU using
> $ gcc --version
> gcc (GCC) 4.6.3 20120306 (Red Hat 4.6.3-2)
>
>
>> Found by coccinelle. Hand patched and reviewed.
>> Tested by compilation only.
>>
>> A simplified version of the semantic match that finds this problem is as
>> follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @@
>> identifier struct_name;
>> struct struct_name to;
>> struct struct_name from;
>> expression E;
>> @@
>> -memcpy(&(to), &(from), E);
>> +to = from;
>> // </smpl>
>>
>> Cc: Andy Walls <awalls@md.metrocast.net>
>
> Signed-off-by: Andy Walls <awalls@md.metrocast.net>
>
>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
>> ---
>>  drivers/media/pci/ivtv/ivtv-i2c.c |   12 ++++--------
>>  1 files changed, 4 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/media/pci/ivtv/ivtv-i2c.c b/drivers/media/pci/ivtv/ivtv-i2c.c
>> index d47f41a..27a8466 100644
>> --- a/drivers/media/pci/ivtv/ivtv-i2c.c
>> +++ b/drivers/media/pci/ivtv/ivtv-i2c.c
>> @@ -719,13 +719,10 @@ int init_ivtv_i2c(struct ivtv *itv)
>>               return -ENODEV;
>>       }
>>       if (itv->options.newi2c > 0) {
>> -             memcpy(&itv->i2c_adap, &ivtv_i2c_adap_hw_template,
>> -                    sizeof(struct i2c_adapter));
>> +             itv->i2c_adap = ivtv_i2c_adap_hw_template;
>>       } else {
>> -             memcpy(&itv->i2c_adap, &ivtv_i2c_adap_template,
>> -                    sizeof(struct i2c_adapter));
>> -             memcpy(&itv->i2c_algo, &ivtv_i2c_algo_template,
>> -                    sizeof(struct i2c_algo_bit_data));
>> +             itv->i2c_adap = ivtv_i2c_adap_template;
>> +             itv->i2c_algo = ivtv_i2c_algo_template;
>>       }
>>       itv->i2c_algo.udelay = itv->options.i2c_clock_period / 2;
>>       itv->i2c_algo.data = itv;
>> @@ -735,8 +732,7 @@ int init_ivtv_i2c(struct ivtv *itv)
>>               itv->instance);
>>       i2c_set_adapdata(&itv->i2c_adap, &itv->v4l2_dev);
>>
>> -     memcpy(&itv->i2c_client, &ivtv_i2c_client_template,
>> -            sizeof(struct i2c_client));
>> +     itv->i2c_client = ivtv_i2c_client_template;
>>       itv->i2c_client.adapter = &itv->i2c_adap;
>>       itv->i2c_adap.dev.parent = &itv->pdev->dev;
>>
>
> I looked at the generated assembly with only this last change
> implemented:
>
> $ objdump -h -r -d -l -s orig-ivtv-i2c.o.sav | less
> [...]
>  07e0 00000000 69767476 20696e74 65726e61  ....ivtv interna
>  07f0 6c000000 00000000 00000000 00000000  l...............
>  0800 00000000 00000000 00000000 00000000  ................
>  0810 00000000 00000000 00000000 00000000  ................
>  0820 00000000 00000000 00000000 00000000  ................
>  0830 00000000 00000000 00000000 00000000  ................
> [...]
> init_ivtv_i2c():
> /home/andy/cx18dev/git/media_tree/drivers/media/video/ivtv/ivtv-i2c.c:738
>     13bb:       48 c7 c6 00 00 00 00    mov    $0x0,%rsi
>                         13be: R_X86_64_32S      .rodata+0x7e0
>     13c2:       48 8d bb 30 04 01 00    lea    0x10430(%rbx),%rdi
>     13c9:       b9 5a 00 00 00          mov    $0x5a,%ecx
>     13ce:       f3 48 a5                rep movsq %ds:(%rsi),%es:(%rdi)
>
>
> $ objdump -h -r -d -l -s orig-ivtv-i2c.o.sav | less
> [...]
>  07e0 00000000 69767476 20696e74 65726e61  ....ivtv interna
>  07f0 6c000000 00000000 00000000 00000000  l...............
>  0800 00000000 00000000 00000000 00000000  ................
>  0810 00000000 00000000 00000000 00000000  ................
>  0820 00000000 00000000 00000000 00000000  ................
>  0830 00000000 00000000 00000000 00000000  ................
> [...]
> init_ivtv_i2c():
> /home/andy/cx18dev/git/media_tree/drivers/media/video/ivtv/ivtv-i2c.c:738
>     13bb:       48 8d bb 30 04 01 00    lea    0x10430(%rbx),%rdi
>     13c2:       48 c7 c6 00 00 00 00    mov    $0x0,%rsi
>                         13c5: R_X86_64_32S      .rodata+0x7e0
>     13c9:       b9 5a 00 00 00          mov    $0x5a,%ecx
>     13ce:       f3 48 a5                rep movsq %ds:(%rsi),%es:(%rdi)
>
>
> The generated code is reordered, but essentially identical.  So I guess
> in this instance, the preprocessor defines resolved such that an x86-64
> optimized memcpy() function was not used from the linux kernel source.
>
> Since all of these memcpy()'s are only called once for each board at
> board initialization, performance here really doesn't matter here
> anyway.  (Unless one is insanely trying to shave microseconds off boot
> time :P )
>
> With other memcpy()/assignement_operator replacement patches, you may
> wish to keep performance in mind, if you are patching a frequently
> called function.
>

Thanks for your thorough review on generated assembly.
It's certainly very helpful.

However, IMHO, this kind of memcpy/assignment can't generate
any performance difference, and we shouldn't worry about this
unless it's a very-very-very hot path.

On the other side, am I being too naive? I'd like to hear others opinion.

Again: thanks!

    Ezequiel

[PATCH 16/23] cx88: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/pci/cx88/cx88-cards.c      |    2 +-
 drivers/media/pci/cx88/cx88-i2c.c        |    3 +--
 drivers/media/pci/cx88/cx88-vp3054-i2c.c |    3 +--
 3 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-cards.c b/drivers/media/pci/cx88/cx88-cards.c
index 0c25524..e2e0b8f 100644
--- a/drivers/media/pci/cx88/cx88-cards.c
+++ b/drivers/media/pci/cx88/cx88-cards.c
@@ -3743,7 +3743,7 @@ struct cx88_core *cx88_core_create(struct pci_dev *pci, int nr)
 		cx88_card_list(core, pci);
 	}
 
-	memcpy(&core->board, &cx88_boards[core->boardnr], sizeof(core->board));
+	core->board = cx88_boards[core->boardnr];
 
 	if (!core->board.num_frontends && (core->board.mpeg & CX88_MPEG_DVB))
 		core->board.num_frontends = 1;
diff --git a/drivers/media/pci/cx88/cx88-i2c.c b/drivers/media/pci/cx88/cx88-i2c.c
index de0f1af..cf2d696 100644
--- a/drivers/media/pci/cx88/cx88-i2c.c
+++ b/drivers/media/pci/cx88/cx88-i2c.c
@@ -139,8 +139,7 @@ int cx88_i2c_init(struct cx88_core *core, struct pci_dev *pci)
 	if (i2c_udelay<5)
 		i2c_udelay=5;
 
-	memcpy(&core->i2c_algo, &cx8800_i2c_algo_template,
-	       sizeof(core->i2c_algo));
+	core->i2c_algo = cx8800_i2c_algo_template;
 
 
 	core->i2c_adap.dev.parent = &pci->dev;
diff --git a/drivers/media/pci/cx88/cx88-vp3054-i2c.c b/drivers/media/pci/cx88/cx88-vp3054-i2c.c
index d77f8ec..deede6e 100644
--- a/drivers/media/pci/cx88/cx88-vp3054-i2c.c
+++ b/drivers/media/pci/cx88/cx88-vp3054-i2c.c
@@ -118,8 +118,7 @@ int vp3054_i2c_probe(struct cx8802_dev *dev)
 		return -ENOMEM;
 	dev->vp3054 = vp3054_i2c;
 
-	memcpy(&vp3054_i2c->algo, &vp3054_i2c_algo_template,
-	       sizeof(vp3054_i2c->algo));
+	vp3054_i2c->algo = vp3054_i2c_algo_template;
 
 	vp3054_i2c->adap.dev.parent = &dev->pci->dev;
 	strlcpy(vp3054_i2c->adap.name, core->name,
-- 
1.7.4.4

[PATCH 17/23] cx23885: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/pci/cx23885/cx23885-video.c |    3 +--
 drivers/media/pci/cx23885/cx23888-ir.c    |    6 ++----
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index 1a21926..62be144 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -1818,8 +1818,7 @@ int cx23885_video_register(struct cx23885_dev *dev)
 	spin_lock_init(&dev->slock);
 
 	/* Initialize VBI template */
-	memcpy(&cx23885_vbi_template, &cx23885_video_template,
-		sizeof(cx23885_vbi_template));
+	cx23885_vbi_template = cx23885_video_template;
 	strcpy(cx23885_vbi_template.name, "cx23885-vbi");
 
 	dev->tvnorm = cx23885_video_template.current_norm;
diff --git a/drivers/media/pci/cx23885/cx23888-ir.c b/drivers/media/pci/cx23885/cx23888-ir.c
index c2bc39c..e448146 100644
--- a/drivers/media/pci/cx23885/cx23888-ir.c
+++ b/drivers/media/pci/cx23885/cx23888-ir.c
@@ -1236,13 +1236,11 @@ int cx23888_ir_probe(struct cx23885_dev *dev)
 		cx23888_ir_write4(dev, CX23888_IR_IRQEN_REG, 0);
 
 		mutex_init(&state->rx_params_lock);
-		memcpy(&default_params, &default_rx_params,
-		       sizeof(struct v4l2_subdev_ir_parameters));
+		default_params = default_rx_params;
 		v4l2_subdev_call(sd, ir, rx_s_parameters, &default_params);
 
 		mutex_init(&state->tx_params_lock);
-		memcpy(&default_params, &default_tx_params,
-		       sizeof(struct v4l2_subdev_ir_parameters));
+		default_params = default_tx_params;
 		v4l2_subdev_call(sd, ir, tx_s_parameters, &default_params);
 	} else {
 		kfifo_free(&state->rx_kfifo);
-- 
1.7.4.4

Re: [PATCH 17/23] cx23885: Replace memcpy with struct assignment

[Andy Walls]

On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
> 
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>

This patch looks OK to me.  You forgot to CC: Steven Toth and/or Devin
Heitmueller (I can't remember who did the VBI work.)

For cx23885-video.c:
Reviewed-by: Andy Walls <awalls@md.metrocast.net>

For cx23885-ir.c:
Signed-off-by: Andy Walls <awalls@md.metrocast.net>


> ---
>  drivers/media/pci/cx23885/cx23885-video.c |    3 +--
>  drivers/media/pci/cx23885/cx23888-ir.c    |    6 ++----
>  2 files changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
> index 1a21926..62be144 100644
> --- a/drivers/media/pci/cx23885/cx23885-video.c
> +++ b/drivers/media/pci/cx23885/cx23885-video.c
> @@ -1818,8 +1818,7 @@ int cx23885_video_register(struct cx23885_dev *dev)
>  	spin_lock_init(&dev->slock);
>  
>  	/* Initialize VBI template */
> -	memcpy(&cx23885_vbi_template, &cx23885_video_template,
> -		sizeof(cx23885_vbi_template));
> +	cx23885_vbi_template = cx23885_video_template;
>  	strcpy(cx23885_vbi_template.name, "cx23885-vbi");
>  
>  	dev->tvnorm = cx23885_video_template.current_norm;
> diff --git a/drivers/media/pci/cx23885/cx23888-ir.c b/drivers/media/pci/cx23885/cx23888-ir.c
> index c2bc39c..e448146 100644
> --- a/drivers/media/pci/cx23885/cx23888-ir.c
> +++ b/drivers/media/pci/cx23885/cx23888-ir.c
> @@ -1236,13 +1236,11 @@ int cx23888_ir_probe(struct cx23885_dev *dev)
>  		cx23888_ir_write4(dev, CX23888_IR_IRQEN_REG, 0);
>  
>  		mutex_init(&state->rx_params_lock);
> -		memcpy(&default_params, &default_rx_params,
> -		       sizeof(struct v4l2_subdev_ir_parameters));
> +		default_params = default_rx_params;
>  		v4l2_subdev_call(sd, ir, rx_s_parameters, &default_params);
>  
>  		mutex_init(&state->tx_params_lock);
> -		memcpy(&default_params, &default_tx_params,
> -		       sizeof(struct v4l2_subdev_ir_parameters));
> +		default_params = default_tx_params;
>  		v4l2_subdev_call(sd, ir, tx_s_parameters, &default_params);
>  	} else {
>  		kfifo_free(&state->rx_kfifo);

Re: [PATCH 17/23] cx23885: Replace memcpy with struct assignment

[Ezequiel Garcia]

(CCed Steven Toth and Devin Heitmueller)

On Tue, Oct 23, 2012 at 7:16 PM, Andy Walls <awalls@md.metrocast.net> wrote:
> On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
>> This kind of memcpy() is error-prone. Its replacement with a struct
>> assignment is prefered because it's type-safe and much easier to read.
>>
>> Found by coccinelle. Hand patched and reviewed.
>> Tested by compilation only.
>>
>> A simplified version of the semantic match that finds this problem is as
>> follows: (http://coccinelle.lip6.fr/)
>>
>> // <smpl>
>> @@
>> identifier struct_name;
>> struct struct_name to;
>> struct struct_name from;
>> expression E;
>> @@
>> -memcpy(&(to), &(from), E);
>> +to = from;
>> // </smpl>
>>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
>
> This patch looks OK to me.  You forgot to CC: Steven Toth and/or Devin
> Heitmueller (I can't remember who did the VBI work.)

Done, thank you.

    Ezequiel

[PATCH 18/23] cx18: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Cc: Andy Walls <awalls@md.metrocast.net>
Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/pci/cx18/cx18-i2c.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/pci/cx18/cx18-i2c.c b/drivers/media/pci/cx18/cx18-i2c.c
index 51609d5..930d40f 100644
--- a/drivers/media/pci/cx18/cx18-i2c.c
+++ b/drivers/media/pci/cx18/cx18-i2c.c
@@ -240,15 +240,13 @@ int init_cx18_i2c(struct cx18 *cx)
 
 	for (i = 0; i < 2; i++) {
 		/* Setup algorithm for adapter */
-		memcpy(&cx->i2c_algo[i], &cx18_i2c_algo_template,
-			sizeof(struct i2c_algo_bit_data));
+		cx->i2c_algo[i] = cx18_i2c_algo_template;
 		cx->i2c_algo_cb_data[i].cx = cx;
 		cx->i2c_algo_cb_data[i].bus_index = i;
 		cx->i2c_algo[i].data = &cx->i2c_algo_cb_data[i];
 
 		/* Setup adapter */
-		memcpy(&cx->i2c_adap[i], &cx18_i2c_adap_template,
-			sizeof(struct i2c_adapter));
+		cx->i2c_adap[i] = cx18_i2c_adap_template;
 		cx->i2c_adap[i].algo_data = &cx->i2c_algo[i];
 		sprintf(cx->i2c_adap[i].name + strlen(cx->i2c_adap[i].name),
 				" #%d-%d", cx->instance, i);
-- 
1.7.4.4

Re: [PATCH 18/23] cx18: Replace memcpy with struct assignment

[Andy Walls]

On Tue, 2012-10-23 at 16:57 -0300, Ezequiel Garcia wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
> 
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
> 
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 
> Cc: Andy Walls <awalls@md.metrocast.net>

Signed-off-by: Andy Walls <awalls@md.metrocast.net>

> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/pci/cx18/cx18-i2c.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/pci/cx18/cx18-i2c.c b/drivers/media/pci/cx18/cx18-i2c.c
> index 51609d5..930d40f 100644
> --- a/drivers/media/pci/cx18/cx18-i2c.c
> +++ b/drivers/media/pci/cx18/cx18-i2c.c
> @@ -240,15 +240,13 @@ int init_cx18_i2c(struct cx18 *cx)
>  
>  	for (i = 0; i < 2; i++) {
>  		/* Setup algorithm for adapter */
> -		memcpy(&cx->i2c_algo[i], &cx18_i2c_algo_template,
> -			sizeof(struct i2c_algo_bit_data));
> +		cx->i2c_algo[i] = cx18_i2c_algo_template;
>  		cx->i2c_algo_cb_data[i].cx = cx;
>  		cx->i2c_algo_cb_data[i].bus_index = i;
>  		cx->i2c_algo[i].data = &cx->i2c_algo_cb_data[i];
>  
>  		/* Setup adapter */
> -		memcpy(&cx->i2c_adap[i], &cx18_i2c_adap_template,
> -			sizeof(struct i2c_adapter));
> +		cx->i2c_adap[i] = cx18_i2c_adap_template;
>  		cx->i2c_adap[i].algo_data = &cx->i2c_algo[i];
>  		sprintf(cx->i2c_adap[i].name + strlen(cx->i2c_adap[i].name),
>  				" #%d-%d", cx->instance, i);

[PATCH 19/23] bttv: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/pci/bt8xx/bttv-i2c.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/bt8xx/bttv-i2c.c b/drivers/media/pci/bt8xx/bttv-i2c.c
index 580c8e6..7398afa 100644
--- a/drivers/media/pci/bt8xx/bttv-i2c.c
+++ b/drivers/media/pci/bt8xx/bttv-i2c.c
@@ -366,8 +366,7 @@ int __devinit init_bttv_i2c(struct bttv *btv)
 
 		strlcpy(btv->c.i2c_adap.name, "bttv",
 			sizeof(btv->c.i2c_adap.name));
-		memcpy(&btv->i2c_algo, &bttv_i2c_algo_bit_template,
-		       sizeof(bttv_i2c_algo_bit_template));
+		btv->i2c_algo = bttv_i2c_algo_bit_template;
 		btv->i2c_algo.udelay = i2c_udelay;
 		btv->i2c_algo.data = btv;
 		btv->c.i2c_adap.algo_data = &btv->i2c_algo;
-- 
1.7.4.4

[PATCH 20/23] dvb-core: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/dvb-core/dvb_frontend.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/dvb-core/dvb_frontend.c b/drivers/media/dvb-core/dvb_frontend.c
index 7e92793..c770464 100644
--- a/drivers/media/dvb-core/dvb_frontend.c
+++ b/drivers/media/dvb-core/dvb_frontend.c
@@ -2256,7 +2256,7 @@ static int dvb_frontend_ioctl_legacy(struct file *file,
 				printk("%s switch command: 0x%04lx\n", __func__, swcmd);
 			do_gettimeofday(&nexttime);
 			if (dvb_frontend_debug)
-				memcpy(&tv[0], &nexttime, sizeof(struct timeval));
+				tv[0] = nexttime;
 			/* before sending a command, initialize by sending
 			 * a 32ms 18V to the switch
 			 */
-- 
1.7.4.4

[PATCH 21/23] dvb-frontends: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/dvb-frontends/cx24116.c   |    2 +-
 drivers/media/dvb-frontends/drxd_hard.c |    5 ++---
 drivers/media/dvb-frontends/stv0299.c   |    2 +-
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/dvb-frontends/cx24116.c b/drivers/media/dvb-frontends/cx24116.c
index b488791..2916d7c 100644
--- a/drivers/media/dvb-frontends/cx24116.c
+++ b/drivers/media/dvb-frontends/cx24116.c
@@ -819,7 +819,7 @@ static int cx24116_read_ucblocks(struct dvb_frontend *fe, u32 *ucblocks)
 static void cx24116_clone_params(struct dvb_frontend *fe)
 {
 	struct cx24116_state *state = fe->demodulator_priv;
-	memcpy(&state->dcur, &state->dnxt, sizeof(state->dcur));
+	state->dcur = state->dnxt;
 }
 
 /* Wait for LNB */
diff --git a/drivers/media/dvb-frontends/drxd_hard.c b/drivers/media/dvb-frontends/drxd_hard.c
index 6d98537..dca1752 100644
--- a/drivers/media/dvb-frontends/drxd_hard.c
+++ b/drivers/media/dvb-frontends/drxd_hard.c
@@ -2963,7 +2963,7 @@ struct dvb_frontend *drxd_attach(const struct drxd_config *config,
 		return NULL;
 	memset(state, 0, sizeof(*state));
 
-	memcpy(&state->ops, &drxd_ops, sizeof(struct dvb_frontend_ops));
+	state->ops = drxd_ops;
 	state->dev = dev;
 	state->config = *config;
 	state->i2c = i2c;
@@ -2974,8 +2974,7 @@ struct dvb_frontend *drxd_attach(const struct drxd_config *config,
 	if (Read16(state, 0, 0, 0) < 0)
 		goto error;
 
-	memcpy(&state->frontend.ops, &drxd_ops,
-	       sizeof(struct dvb_frontend_ops));
+	state->frontend.ops = drxd_ops;
 	state->frontend.demodulator_priv = state;
 	ConfigureMPEGOutput(state, 0);
 	return &state->frontend;
diff --git a/drivers/media/dvb-frontends/stv0299.c b/drivers/media/dvb-frontends/stv0299.c
index 92a6075..b57ecf4 100644
--- a/drivers/media/dvb-frontends/stv0299.c
+++ b/drivers/media/dvb-frontends/stv0299.c
@@ -420,7 +420,7 @@ static int stv0299_send_legacy_dish_cmd (struct dvb_frontend* fe, unsigned long
 
 	do_gettimeofday (&nexttime);
 	if (debug_legacy_dish_switch)
-		memcpy (&tv[0], &nexttime, sizeof (struct timeval));
+		tv[0] = nexttime;
 	stv0299_writeregI (state, 0x0c, reg0x0c | 0x50); /* set LNB to 18V */
 
 	dvb_frontend_sleep_until(&nexttime, 32000);
-- 
1.7.4.4

[PATCH 22/23] radio-wl1273: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/radio/radio-wl1273.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/radio/radio-wl1273.c b/drivers/media/radio/radio-wl1273.c
index 9b0c9fa..6e55e93 100644
--- a/drivers/media/radio/radio-wl1273.c
+++ b/drivers/media/radio/radio-wl1273.c
@@ -2084,8 +2084,7 @@ static int __devinit wl1273_fm_radio_probe(struct platform_device *pdev)
 	}
 
 	/* V4L2 configuration */
-	memcpy(&radio->videodev, &wl1273_viddev_template,
-	       sizeof(wl1273_viddev_template));
+	radio->videodev = wl1273_viddev_template;
 
 	radio->videodev.v4l2_dev = &radio->v4l2dev;
 
-- 
1.7.4.4

[PATCH 23/23] wl128x: Replace memcpy with struct assignment

[Ezequiel Garcia]

This kind of memcpy() is error-prone. Its replacement with a struct
assignment is prefered because it's type-safe and much easier to read.

Found by coccinelle. Hand patched and reviewed.
Tested by compilation only.

A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier struct_name;
struct struct_name to;
struct struct_name from;
expression E;
@@
-memcpy(&(to), &(from), E);
+to = from;
// </smpl>

Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 drivers/media/radio/wl128x/fmdrv_common.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/media/radio/wl128x/fmdrv_common.c b/drivers/media/radio/wl128x/fmdrv_common.c
index bf867a6..902f19d 100644
--- a/drivers/media/radio/wl128x/fmdrv_common.c
+++ b/drivers/media/radio/wl128x/fmdrv_common.c
@@ -1563,8 +1563,7 @@ int fmc_prepare(struct fmdev *fmdev)
 	fmdev->irq_info.mask = FM_MAL_EVENT;
 
 	/* Region info */
-	memcpy(&fmdev->rx.region, &region_configs[default_radio_region],
-			sizeof(struct region_info));
+	fmdev->rx.region = region_configs[default_radio_region];
 
 	fmdev->rx.mute_mode = FM_MUTE_OFF;
 	fmdev->rx.rf_depend_mute = FM_RX_RF_DEPENDENT_MUTE_OFF;
-- 
1.7.4.4

Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment

[Laurent Pinchart]

Hi Ezequiel,

Thanks for the patch.

On Tuesday 23 October 2012 16:57:04 Ezequiel Garcia wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
> 
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.

This looks good, but there's one more memcpy that can be replaced by a direct 
structure assignment in uvc_ctrl_add_info() 
(drivers/media/usb/uvc/uvc_ctrl.c). You might want to check why it hasn't been 
caught by the semantic patch.

> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
> 
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
> 
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..4fc8737 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming
> *stream, goto done;
>  	}
> 
> -	memcpy(&stream->ctrl, &probe, sizeof probe);
> +	stream->ctrl = probe;
>  	stream->cur_format = format;
>  	stream->cur_frame = frame;
> 
> @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, return -EBUSY;
>  	}
> 
> -	memcpy(&probe, &stream->ctrl, sizeof probe);
> +	probe = stream->ctrl;
>  	probe.dwFrameInterval =
>  		uvc_try_frame_interval(stream->cur_frame, interval);
> 
> @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming
> *stream, return ret;
>  	}
> 
> -	memcpy(&stream->ctrl, &probe, sizeof probe);
> +	stream->ctrl = probe;
>  	mutex_unlock(&stream->mutex);
> 
>  	/* Return the actual frame period. */

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment

[Andy Walls]

Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

>Hi Ezequiel,
>
>Thanks for the patch.
>
>On Tuesday 23 October 2012 16:57:04 Ezequiel Garcia wrote:
>> This kind of memcpy() is error-prone. Its replacement with a struct
>> assignment is prefered because it's type-safe and much easier to
>read.
>> 
>> Found by coccinelle. Hand patched and reviewed.
>> Tested by compilation only.
>
>This looks good, but there's one more memcpy that can be replaced by a
>direct 
>structure assignment in uvc_ctrl_add_info() 
>(drivers/media/usb/uvc/uvc_ctrl.c). You might want to check why it
>hasn't been 
>caught by the semantic patch.
>
>> A simplified version of the semantic match that finds this problem is
>as
>> follows: (http://coccinelle.lip6.fr/)
>> 
>> // <smpl>
>> @@
>> identifier struct_name;
>> struct struct_name to;
>> struct struct_name from;
>> expression E;
>> @@
>> -memcpy(&(to), &(from), E);
>> +to = from;
>> // </smpl>
>> 
>> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
>> ---
>>  drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
>>  1 files changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>> b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..4fc8737 100644
>> --- a/drivers/media/usb/uvc/uvc_v4l2.c
>> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct
>uvc_streaming
>> *stream, goto done;
>>  	}
>> 
>> -	memcpy(&stream->ctrl, &probe, sizeof probe);
>> +	stream->ctrl = probe;
>>  	stream->cur_format = format;
>>  	stream->cur_frame = frame;
>> 
>> @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct
>uvc_streaming
>> *stream, return -EBUSY;
>>  	}
>> 
>> -	memcpy(&probe, &stream->ctrl, sizeof probe);
>> +	probe = stream->ctrl;
>>  	probe.dwFrameInterval =
>>  		uvc_try_frame_interval(stream->cur_frame, interval);
>> 
>> @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct
>uvc_streaming
>> *stream, return ret;
>>  	}
>> 
>> -	memcpy(&stream->ctrl, &probe, sizeof probe);
>> +	stream->ctrl = probe;
>>  	mutex_unlock(&stream->mutex);
>> 
>>  	/* Return the actual frame period. */
>
>-- 
>Regards,
>
>Laurent Pinchart
>
>--
>To unsubscribe from this list: send the line "unsubscribe linux-media"
>in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Maybe because there is no '&' operator on the second argument. 

Regards,
Andy

Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment

[Ezequiel Garcia]

Mauro,

On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> This kind of memcpy() is error-prone. Its replacement with a struct
> assignment is prefered because it's type-safe and much easier to read.
>
> Found by coccinelle. Hand patched and reviewed.
> Tested by compilation only.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @@
> identifier struct_name;
> struct struct_name to;
> struct struct_name from;
> expression E;
> @@
> -memcpy(&(to), &(from), E);
> +to = from;
> // </smpl>
>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> ---
>  drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> index f00db30..4fc8737 100644
> --- a/drivers/media/usb/uvc/uvc_v4l2.c
> +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming *stream,
>                 goto done;
>         }
>
> -       memcpy(&stream->ctrl, &probe, sizeof probe);
> +       stream->ctrl = probe;
>         stream->cur_format = format;
>         stream->cur_frame = frame;
>
> @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
>                 return -EBUSY;
>         }
>
> -       memcpy(&probe, &stream->ctrl, sizeof probe);
> +       probe = stream->ctrl;
>         probe.dwFrameInterval =
>                 uvc_try_frame_interval(stream->cur_frame, interval);
>
> @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
>                 return ret;
>         }
>
> -       memcpy(&stream->ctrl, &probe, sizeof probe);
> +       stream->ctrl = probe;
>         mutex_unlock(&stream->mutex);
>
>         /* Return the actual frame period. */
> --
> 1.7.4.4
>

It seems you've marked this one as "Changes requested" [1].
However, Laurent didn't request any change,
but just pointed out we missed one memcpy replacement candidate.

I believe it's safe to apply the patch (together with the other 20 patches)
and we can fix the missing spot in another patch.

Thanks,

-- 
    Ezequiel

[1] http://patchwork.linuxtv.org/patch/15142/

Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment

[Mauro Carvalho Chehab]

Em Thu, 27 Dec 2012 18:12:46 -0300
Ezequiel Garcia <elezegarcia@gmail.com> escreveu:

> Mauro,
> 
> On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia <elezegarcia@gmail.com> wrote:
> > This kind of memcpy() is error-prone. Its replacement with a struct
> > assignment is prefered because it's type-safe and much easier to read.
> >
> > Found by coccinelle. Hand patched and reviewed.
> > Tested by compilation only.
> >
> > A simplified version of the semantic match that finds this problem is as
> > follows: (http://coccinelle.lip6.fr/)
> >
> > // <smpl>
> > @@
> > identifier struct_name;
> > struct struct_name to;
> > struct struct_name from;
> > expression E;
> > @@
> > -memcpy(&(to), &(from), E);
> > +to = from;
> > // </smpl>
> >
> > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> > Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> > ---
> >  drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
> > index f00db30..4fc8737 100644
> > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming *stream,
> >                 goto done;
> >         }
> >
> > -       memcpy(&stream->ctrl, &probe, sizeof probe);
> > +       stream->ctrl = probe;
> >         stream->cur_format = format;
> >         stream->cur_frame = frame;
> >
> > @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
> >                 return -EBUSY;
> >         }
> >
> > -       memcpy(&probe, &stream->ctrl, sizeof probe);
> > +       probe = stream->ctrl;
> >         probe.dwFrameInterval =
> >                 uvc_try_frame_interval(stream->cur_frame, interval);
> >
> > @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct uvc_streaming *stream,
> >                 return ret;
> >         }
> >
> > -       memcpy(&stream->ctrl, &probe, sizeof probe);
> > +       stream->ctrl = probe;
> >         mutex_unlock(&stream->mutex);
> >
> >         /* Return the actual frame period. */
> > --
> > 1.7.4.4
> >
> 
> It seems you've marked this one as "Changes requested" [1].
> However, Laurent didn't request any change,
> but just pointed out we missed one memcpy replacement candidate.
> 
> I believe it's safe to apply the patch (together with the other 20 patches)
> and we can fix the missing spot in another patch.

The other patches got applied already. Well just do whatever Laurent asked you
and re-submit this one ;)

Regards,
Mauro

Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment

[Laurent Pinchart]

Hi Ezequiel,

On Thursday 27 December 2012 21:49:37 Mauro Carvalho Chehab wrote:
> Em Thu, 27 Dec 2012 18:12:46 -0300 Ezequiel Garcia escreveu:
> > On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia wrote:
> > > This kind of memcpy() is error-prone. Its replacement with a struct
> > > assignment is prefered because it's type-safe and much easier to read.
> > > 
> > > Found by coccinelle. Hand patched and reviewed.
> > > Tested by compilation only.
> > > 
> > > A simplified version of the semantic match that finds this problem is as
> > > follows: (http://coccinelle.lip6.fr/)
> > > 
> > > // <smpl>
> > > @@
> > > identifier struct_name;
> > > struct struct_name to;
> > > struct struct_name from;
> > > expression E;
> > > @@
> > > -memcpy(&(to), &(from), E);
> > > +to = from;
> > > // </smpl>
> > > 
> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
> > > Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
> > > ---
> > > 
> > >  drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
> > >  1 files changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
> > > b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..4fc8737 100644
> > > --- a/drivers/media/usb/uvc/uvc_v4l2.c
> > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
> > > @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming
> > > *stream,
> > >                 goto done;
> > >         }
> > > 
> > > -       memcpy(&stream->ctrl, &probe, sizeof probe);
> > > +       stream->ctrl = probe;
> > > 
> > >         stream->cur_format = format;
> > >         stream->cur_frame = frame;
> > > @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct
> > > uvc_streaming *stream,
> > >                 return -EBUSY;
> > >         }
> > > 
> > > -       memcpy(&probe, &stream->ctrl, sizeof probe);
> > > +       probe = stream->ctrl;
> > >         probe.dwFrameInterval =
> > >                 uvc_try_frame_interval(stream->cur_frame, interval);
> > > 
> > > @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct
> > > uvc_streaming *stream,
> > >                 return ret;
> > >         }
> > > 
> > > -       memcpy(&stream->ctrl, &probe, sizeof probe);
> > > +       stream->ctrl = probe;
> > >         mutex_unlock(&stream->mutex);
> > >         
> > >         /* Return the actual frame period. */
> > 
> > It seems you've marked this one as "Changes requested" [1].
> > However, Laurent didn't request any change,
> > but just pointed out we missed one memcpy replacement candidate.
> > 
> > I believe it's safe to apply the patch (together with the other 20
> > patches) and we can fix the missing spot in another patch.
> 
> The other patches got applied already. Well just do whatever Laurent asked
> you and re-submit this one ;)

Could you please resubmit this patch with the missed memcpy replaced by a 
struct assignment ? I'll then add it to my tree.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/23] uvc: Replace memcpy with struct assignment

[Ezequiel Garcia]

On Wed, Jan 9, 2013 at 9:19 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Ezequiel,
>
> On Thursday 27 December 2012 21:49:37 Mauro Carvalho Chehab wrote:
>> Em Thu, 27 Dec 2012 18:12:46 -0300 Ezequiel Garcia escreveu:
>> > On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia wrote:
>> > > This kind of memcpy() is error-prone. Its replacement with a struct
>> > > assignment is prefered because it's type-safe and much easier to read.
>> > >
>> > > Found by coccinelle. Hand patched and reviewed.
>> > > Tested by compilation only.
>> > >
>> > > A simplified version of the semantic match that finds this problem is as
>> > > follows: (http://coccinelle.lip6.fr/)
>> > >
>> > > // <smpl>
>> > > @@
>> > > identifier struct_name;
>> > > struct struct_name to;
>> > > struct struct_name from;
>> > > expression E;
>> > > @@
>> > > -memcpy(&(to), &(from), E);
>> > > +to = from;
>> > > // </smpl>
>> > >
>> > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> > > Signed-off-by: Peter Senna Tschudin <peter.senna@gmail.com>
>> > > Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
>> > > ---
>> > >
>> > >  drivers/media/usb/uvc/uvc_v4l2.c |    6 +++---
>> > >  1 files changed, 3 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/media/usb/uvc/uvc_v4l2.c
>> > > b/drivers/media/usb/uvc/uvc_v4l2.c index f00db30..4fc8737 100644
>> > > --- a/drivers/media/usb/uvc/uvc_v4l2.c
>> > > +++ b/drivers/media/usb/uvc/uvc_v4l2.c
>> > > @@ -314,7 +314,7 @@ static int uvc_v4l2_set_format(struct uvc_streaming
>> > > *stream,
>> > >                 goto done;
>> > >         }
>> > >
>> > > -       memcpy(&stream->ctrl, &probe, sizeof probe);
>> > > +       stream->ctrl = probe;
>> > >
>> > >         stream->cur_format = format;
>> > >         stream->cur_frame = frame;
>> > > @@ -386,7 +386,7 @@ static int uvc_v4l2_set_streamparm(struct
>> > > uvc_streaming *stream,
>> > >                 return -EBUSY;
>> > >         }
>> > >
>> > > -       memcpy(&probe, &stream->ctrl, sizeof probe);
>> > > +       probe = stream->ctrl;
>> > >         probe.dwFrameInterval =
>> > >                 uvc_try_frame_interval(stream->cur_frame, interval);
>> > >
>> > > @@ -397,7 +397,7 @@ static int uvc_v4l2_set_streamparm(struct
>> > > uvc_streaming *stream,
>> > >                 return ret;
>> > >         }
>> > >
>> > > -       memcpy(&stream->ctrl, &probe, sizeof probe);
>> > > +       stream->ctrl = probe;
>> > >         mutex_unlock(&stream->mutex);
>> > >
>> > >         /* Return the actual frame period. */
>> >
>> > It seems you've marked this one as "Changes requested" [1].
>> > However, Laurent didn't request any change,
>> > but just pointed out we missed one memcpy replacement candidate.
>> >
>> > I believe it's safe to apply the patch (together with the other 20
>> > patches) and we can fix the missing spot in another patch.
>>
>> The other patches got applied already. Well just do whatever Laurent asked
>> you and re-submit this one ;)
>
> Could you please resubmit this patch with the missed memcpy replaced by a
> struct assignment ? I'll then add it to my tree.
>

Sure!

Thanks,

-- 
    Ezequiel