LinuxLists.cc - [PATCH 01/23] uvc: Replace memcpy with struct assignment

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 <[email protected]>
Signed-off-by: Peter Senna Tschudin <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2012-10-23 19:59:03

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 19:59:15

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 19:59:08

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 19:59:31

by Ezequiel Garcia

[permalink] [raw]

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

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2012-10-23 19:59:39

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 19:59:37

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 19:59:27

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 19:59:24

by Ezequiel Garcia

[permalink] [raw]

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

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2012-10-23 20:00:45

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 20:01:27

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 20:01:53

by Ezequiel Garcia

[permalink] [raw]

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

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2012-10-23 20:02:26

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 20:03:49

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 20:04:20

by Ezequiel Garcia

[permalink] [raw]

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

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 <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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

2012-10-23 21:17:06

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 21:17:39

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 21:18:15

by Ezequiel Garcia

[permalink] [raw]

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

2012-10-23 22:09:46

by Andy Walls

[permalink] [raw]

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

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 <[email protected]>

Signed-off-by: Andy Walls <[email protected]>

> Signed-off-by: Peter Senna Tschudin <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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

2012-10-23 22:16:28

by Andy Walls

[permalink] [raw]

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

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 <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>

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 <[email protected]>

For cx23885-ir.c:
Signed-off-by: Andy Walls <[email protected]>

> ---
> 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);

2012-10-23 22:17:39

by Andy Walls

[permalink] [raw]

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

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 <[email protected]>

Signed-off-by: Andy Walls <[email protected]>

> Signed-off-by: Peter Senna Tschudin <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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);

2012-10-23 22:20:39

by Andy Walls

[permalink] [raw]

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

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 <[email protected]>

> Signed-off-by: Peter Senna Tschudin <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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;

2012-10-23 22:36:49

by Andy Walls

[permalink] [raw]

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

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 <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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

2012-10-24 00:29:00

by Ezequiel Garcia

[permalink] [raw]

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

Hey Andy,

On Tue, Oct 23, 2012 at 7:08 PM, Andy Walls <[email protected]> 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 <[email protected]>
>
> Signed-off-by: Andy Walls <[email protected]>
>
>
>> Signed-off-by: Peter Senna Tschudin <[email protected]>
>> Signed-off-by: Ezequiel Garcia <[email protected]>
>> ---
>> 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

2012-10-24 00:50:30

by Ezequiel Garcia

[permalink] [raw]

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

On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia <[email protected]> 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 <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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

2012-10-24 00:52:19

by Ezequiel Garcia

[permalink] [raw]

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

(CCed Steven Toth and Devin Heitmueller)

On Tue, Oct 23, 2012 at 7:16 PM, Andy Walls <[email protected]> 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 <[email protected]>
>> Signed-off-by: Ezequiel Garcia <[email protected]>
>
> 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

2012-10-24 22:34:43

by Laurent Pinchart

[permalink] [raw]

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

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 <[email protected]>
> Signed-off-by: Peter Senna Tschudin <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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

2012-10-24 23:10:53

by Andy Walls

[permalink] [raw]

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

Laurent Pinchart <[email protected]> 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 <[email protected]>
>> Signed-off-by: Peter Senna Tschudin <[email protected]>
>> Signed-off-by: Ezequiel Garcia <[email protected]>
>> ---
>> 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 [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html

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

Regards,
Andy