On Tuesday 11 August 2009 17:08:27 Hans Verkuil wrote: > Hi Mauro, > > > Em Thu, 6 Aug 2009 22:04:13 +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. > >> - v4l2-spec: document the new string control type. > >> - v4l2-ctl: modulator bug fixes > >> - v4l2-ctl: add support for string controls > >> > >> It's the same as the previous version, except for reverting the variable > >> renaming and for improving the comments above the ctrl_is_value64 and > >> ctrl_is_pointer functions. I've decided to keep the ctrl_is_value64() > >> function because I want to keep the correct value64 conversion code in > >> compat32 rather than having to do that work again at some point in the > >> future. > > > > I agree with Trent, this really sucks: > > > > static inline int ctrl_is_value64(u32 id) > > { > > - return 0; > > + switch (id) { > > + case V4L2_CID_RDS_TX_PS_NAME: > > + case V4L2_CID_RDS_TX_RADIO_TEXT: > > + return 1; > > + default: > > + return 0; > > + } > > } > > > > Yet, by looking on how the struct is defined: > > > > struct v4l2_ext_control { > > __u32 id; > > __u32 reserved2[2]; > > union { > > __s32 value; > > __s64 value64; > > void *reserved; > > }; > > } __attribute__ ((packed)); > > > > Except on the strings case, I don't see why we need a compat stuff for > > this > > routine at the first place, copying field by field. Correct me if I'm > > wrong, > > but, on both 32 and 64 bit cases, the fields will be at the same position > > on > > both 32 and 64 bits. So, the code could be just: > > > > copy_in_user(&kcontrols, &ucontrols, sizeof(ucontrols)); > > if (ctrl_is_pointer(id)) { > > /* get_user/compat_ptr magic */ > > } > > OK, makes sense. <snip> > I will prepare a new pull request where I use copy_in_user. While working on this I found additional problems with the way a string control is copied in the compat layer. I have what I think is the right solution in my v4l-dvb tree, but I want to do some testing of the compat layer first before I make the pull request. Regards, Hans -- Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom -- 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
