Re: [PATCH v2] v4l2-ctrls: Add v4l2_ctrl_[gs]_ctrl_int64()

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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@ideasonboard.com>

[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@cisco.com>

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