Re: [PATCH 04/23] sn9c102: Replace memcpy with struct assignment
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/) > > // > @@ > identifier struct_name; > struct struct_name to; > struct struct_name from; > expression E; > @@ > -memcpy(&(to), &(from), E); > +to = from; > // > > Signed-off-by: Peter Senna Tschudin > Signed-off-by: Ezequiel Garcia > --- > 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(, >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(, >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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/23] sn9c102: 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/) // @@ identifier struct_name; struct struct_name to; struct struct_name from; expression E; @@ -memcpy(&(to), &(from), E); +to = from; // Signed-off-by: Peter Senna Tschudin Signed-off-by: Ezequiel Garcia --- 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(, >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(, >buf, sizeof(b)); + b = f->buf; if (f->vma_use_count) b.flags |= V4L2_BUF_FLAG_MAPPED; -- 1.7.4.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 04/23] sn9c102: 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 peter.se...@gmail.com Signed-off-by: Ezequiel Garcia elezegar...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 04/23] sn9c102: Replace memcpy with struct assignment
On Tue, Oct 23, 2012 at 4:57 PM, Ezequiel Garcia elezegar...@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.se...@gmail.com Signed-off-by: Ezequiel Garcia elezegar...@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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
