On 03/12/14 12:17, Hans Verkuil wrote:
> Hi Sylwester,
>
> On 12/03/14 12:14, Sylwester Nawrocki wrote:
>> > Hi Hans,
>> >
>> > On 02/12/14 13:21, Hans Verkuil wrote:
>>> >> -static int s5k6aa_set_crop(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh,
>>> >> - struct v4l2_subdev_crop *crop)
>>> >> +static int s5k6aa_set_selection(struct v4l2_subdev *sd,
>>> >> + struct v4l2_subdev_fh *fh,
>>> >> + struct v4l2_subdev_selection *sel)
>>> >> {
>>> >> struct s5k6aa *s5k6aa = to_s5k6aa(sd);
>>> >> struct v4l2_mbus_framefmt *mf;
>>> >> unsigned int max_x, max_y;
>>> >> struct v4l2_rect *crop_r;
>>> >>
>>> >> + if (sel->pad || sel->target != V4L2_SEL_TGT_CROP)
>>> >> + return -EINVAL;
>>> >> +
>> >
>> > Isn't checking sel->pad redundant here ? There is already the pad index
>> > validation in check_selection() in v4l2-subdev.c and this driver has only
>> > one pad.
>
> If it is called from a bridge driver, then it hasn't gone through
> check_selection().
>
> That said, if it is called from a bridge driver, then one might expect
> correct usage of pad.
Indeed, there is still a possibility to have wrong pad index passed
to those functions. I won't object to this patch being merged as is,
even though functional changes could be minimized by not adding a
check which wasn't originally there. :)
Acked-by: Sylwester Nawrocki <s.nawrocki@xxxxxxxxxxx>
--
Regards,
Sylwester
--
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
