Em Sun, 2 Aug 2009 12:10:04 +0200 Hans Verkuil <hverkuil@xxxxxxxxx> escreveu: > Hi Mauro, > > Please pull from http://www.linuxtv.org/hg/~hverkuil/v4l-dvb for the > following: > > - v4l: introduce string control support. Hans, This looks very weird: +/* The following two functions really belong in v4l2-common, but that causes + a circular dependency between modules. We need to think about this, but + for now this will do. */ + +/* Return non-zero if this control is of type VALUE64. + * + * This information is used inside v4l2_compat_ioctl32. */ +static int ctrl_is_value64(u32 id) +{ + return 0; +} + +/* Return non-zero if this control is a pointer type. Currently only + * type STRING is a pointer type. + * + * This information is used inside v4l2_compat_ioctl32. */ +static int ctrl_is_pointer(u32 id) +{ + return 0; +} + ... + } else if (ctrl_is_value64(kctrl->id)) { + if (copy_in_user(&kctrl->value64, &uctrl->value64, + sizeof(uctrl->value64))) + return -EFAULT; ... + if (ctrl_is_value64(kctrl->id)) { + if (copy_in_user(&uctrl->value64, &kctrl->value64, + sizeof(uctrl->value64))) + return -EFAULT; + } else if (!ctrl_is_pointer(kctrl->id) && + copy_in_user(&uctrl->value, &kctrl->value, + sizeof(uctrl->value))) { Why do you need two routines that will always return zero? Why to create a code that will never be used? v4l2-compat-ioctl32.c is already complex enough without adding any bogus code. I noticed that you're also renaming the names of some fields there: - struct v4l2_ext_control __user *ucontrols; - struct v4l2_ext_control __user *kcontrols = kp->controls; + struct v4l2_ext_control __user *uctrl; + struct v4l2_ext_control __user *kctrl = kp->controls; I dunno why you decided to do it, but having this together with the changes at v4l2-compat-ioctl32.c makes the code even harder to understand. Could you please move the v4l2-compat-ioctl32 "cosmetic" changes like above into a separate patch? Cheers, Mauro -- 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
