| Message ID | 1501245205-15802-2-git-send-email-g.liakhovetski@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.liakhovetski@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.pinchart@ideasonboard.com> > *nplanes = 1; > sizes[0] = size; > return 0;
And I forgot to mention, the usual prefix for the subject line is "uvcvideo:" followed by a capitalized word. On Monday 31 Jul 2017 16:57:23 Laurent Pinchart wrote: > 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.liakhovetski@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.pinchart@ideasonboard.com> > > > *nplanes = 1; > > sizes[0] = size; > > return 0;
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. */ if (*nplanes) - return sizes[0] < size ? -EINVAL : 0; + return sizes[0] < size || *nplanes != 1 ? -EINVAL : 0; *nplanes = 1; sizes[0] = size; return 0;
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.liakhovetski@intel.com> --- drivers/media/usb/uvc/uvc_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)