Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxx> >> Signed-off-by: Peter Senna Tschudin <peter.senna@xxxxxxxxx> >> Signed-off-by: Ezequiel Garcia <elezegarcia@xxxxxxxxx> >> --- >> 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@xxxxxxxxxxxxxxx >More majordomo info at http://vger.kernel.org/majordomo-info.html Maybe because there is no '&' operator on the second argument. Regards, Andy -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
