On 12/03/14 12:49, Sylwester Nawrocki wrote:
> 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>
>
I've dropped the sel->pad check.
Regards,
Hans
--
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
