Hi Guennadi,
Thank you for the patch.
On Friday 28 Jul 2017 14:33:20 Guennadi Liakhovetski wrote:
> According to documentation of struct vb2_ops the .queue_setup() callback
> should return an error if the number of planes parameter contains an
> invalid value on input. Fix this instead of ignoring the value.
>
> Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovet...@intel.com>
> ---
> drivers/media/usb/uvc/uvc_queue.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/uvc/uvc_queue.c
> b/drivers/media/usb/uvc/uvc_queue.c index aa21997..371a4ad 100644
> --- a/drivers/media/usb/uvc/uvc_queue.c
> +++ b/drivers/media/usb/uvc/uvc_queue.c
> @@ -84,7 +84,7 @@ static int uvc_queue_setup(struct vb2_queue *vq,
>
> /* Make sure the image size is large enough. */
Nitpicking, I'd update the comment as well.
/*
* When called with plane sizes, validate them. The driver supports
* single planar formats only, and requires buffers to be large enough
* to store a complete frame.
*/
> if (*nplanes)
> - return sizes[0] < size ? -EINVAL : 0;
> + return sizes[0] < size || *nplanes != 1 ? -EINVAL : 0;
Nitpicking again, I'd test *nplanes first, as it conditions which entries of
the sizes array are valid. If course the if (*nplanes) test ensures that entry
0 is valid, so it won't make a difference at runtime, it's just about code
readability.
The patch looks good otherwise,
Reviewed-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
> *nplanes = 1;
> sizes[0] = size;
> return 0;
--
Regards,
Laurent Pinchart
