Hi Hans,
On Monday 23 July 2012 17:05:35 Hans Verkuil wrote:
> On Mon July 23 2012 16:02:40 Laurent Pinchart wrote:
> > These helper functions get and set a 64-bit control's value from within
> > a driver. They are similar to v4l2_ctrl_[gs]_ctrl() but operate on
> > 64-bit integer controls instead of 32-bit controls.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
[snip]
> > -static int get_ctrl(struct v4l2_ctrl *ctrl, s32 *val)
> > +static int get_ctrl(struct v4l2_ctrl *ctrl, struct v4l2_ext_control *c)
> >
> > {
> >
> > struct v4l2_ctrl *master = ctrl->cluster[0];
> > int ret = 0;
> > int i;
> >
> > + /* String controls are not supported. The new_to_user() and
> > + * cur_to_user() calls below would need to be fixed not to access
> > + * userspace memory.
>
> Just one small suggestion: change this comment to:
>
> /* String controls are not supported. The new_to_user() and
> * cur_to_user() calls below would need to be modified not to access
> * userspace memory when called from get_ctrl().
> */
>
> And a similar change in the comment with set_ctrl.
>
> The word 'fixed' suggested that new_to_user etc. were broken, which isn't
> the case. We are just using it in a special situation.
OK. Fixed.
> With the above change to get/set_ctrl you can add my
>
> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
Thank you. I'll resubmit the patch (along with a driver patch that uses it).
Would you like to push it through your tree ?
--
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
