* [PATCH 1/1] v4l: subdev: Unify argument validation across IOCTLs
@ 2014-06-03 9:25 Sakari Ailus
2014-06-03 17:16 ` Laurent Pinchart
0 siblings, 1 reply; 3+ messages in thread
From: Sakari Ailus @ 2014-06-03 9:25 UTC (permalink / raw)
To: linux-media
Separate validation of different argument types. There's no reason to do
this separately for every IOCTL.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
drivers/media/v4l2-core/v4l2-subdev.c | 119 +++++++++++++++++++++-------------
1 file changed, 73 insertions(+), 46 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
index 058c1a6..496f9bc 100644
--- a/drivers/media/v4l2-core/v4l2-subdev.c
+++ b/drivers/media/v4l2-core/v4l2-subdev.c
@@ -126,6 +126,55 @@ static int subdev_close(struct file *file)
return 0;
}
+static int check_format(struct v4l2_subdev *sd,
+ struct v4l2_subdev_format *format)
+{
+ if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
+ format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ if (format->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop *crop)
+{
+ if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
+ crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ if (crop->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int check_selection(struct v4l2_subdev *sd,
+ struct v4l2_subdev_selection *sel)
+{
+ if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
+ sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
+ return -EINVAL;
+
+ if (sel->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid *edid)
+{
+ if (edid->pad >= sd->entity.num_pads)
+ return -EINVAL;
+
+ if (edid->blocks && edid->edid == NULL)
+ return -EINVAL;
+
+ return 0;
+}
+
static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
{
struct video_device *vdev = video_devdata(file);
@@ -202,26 +251,20 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
#if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
case VIDIOC_SUBDEV_G_FMT: {
struct v4l2_subdev_format *format = arg;
+ int rval = check_format(sd, format);
- if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
- format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
- if (format->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh, format);
}
case VIDIOC_SUBDEV_S_FMT: {
struct v4l2_subdev_format *format = arg;
+ int rval = check_format(sd, format);
- if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
- format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
- if (format->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
}
@@ -229,14 +272,10 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
case VIDIOC_SUBDEV_G_CROP: {
struct v4l2_subdev_crop *crop = arg;
struct v4l2_subdev_selection sel;
- int rval;
-
- if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
- crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
+ int rval = check_crop(sd, crop);
- if (crop->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
rval = v4l2_subdev_call(sd, pad, get_crop, subdev_fh, crop);
if (rval != -ENOIOCTLCMD)
@@ -258,14 +297,10 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
case VIDIOC_SUBDEV_S_CROP: {
struct v4l2_subdev_crop *crop = arg;
struct v4l2_subdev_selection sel;
- int rval;
-
- if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
- crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
+ int rval = check_crop(sd, crop);
- if (crop->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
rval = v4l2_subdev_call(sd, pad, set_crop, subdev_fh, crop);
if (rval != -ENOIOCTLCMD)
@@ -335,13 +370,10 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
case VIDIOC_SUBDEV_G_SELECTION: {
struct v4l2_subdev_selection *sel = arg;
+ int rval = check_selection(sd, sel);
- if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
- sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
- if (sel->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(
sd, pad, get_selection, subdev_fh, sel);
@@ -349,13 +381,10 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
case VIDIOC_SUBDEV_S_SELECTION: {
struct v4l2_subdev_selection *sel = arg;
+ int rval = check_selection(sd, sel);
- if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
- sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
- return -EINVAL;
-
- if (sel->pad >= sd->entity.num_pads)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(
sd, pad, set_selection, subdev_fh, sel);
@@ -363,22 +392,20 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
case VIDIOC_G_EDID: {
struct v4l2_subdev_edid *edid = arg;
+ int rval = check_edid(sd, edid);
- if (edid->pad >= sd->entity.num_pads)
- return -EINVAL;
- if (edid->blocks && edid->edid == NULL)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(sd, pad, get_edid, edid);
}
case VIDIOC_S_EDID: {
struct v4l2_subdev_edid *edid = arg;
+ int rval = check_edid(sd, edid);
- if (edid->pad >= sd->entity.num_pads)
- return -EINVAL;
- if (edid->blocks && edid->edid == NULL)
- return -EINVAL;
+ if (rval)
+ return rval;
return v4l2_subdev_call(sd, pad, set_edid, edid);
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] v4l: subdev: Unify argument validation across IOCTLs
2014-06-03 9:25 [PATCH 1/1] v4l: subdev: Unify argument validation across IOCTLs Sakari Ailus
@ 2014-06-03 17:16 ` Laurent Pinchart
2014-06-04 11:17 ` Sakari Ailus
0 siblings, 1 reply; 3+ messages in thread
From: Laurent Pinchart @ 2014-06-03 17:16 UTC (permalink / raw)
To: Sakari Ailus; +Cc: linux-media
Hi Sakari,
Thank you for the patch.
On Tuesday 03 June 2014 12:25:16 Sakari Ailus wrote:
> Separate validation of different argument types. There's no reason to do
> this separately for every IOCTL.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
> drivers/media/v4l2-core/v4l2-subdev.c | 119 ++++++++++++++++++-------------
> 1 file changed, 73 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c
> b/drivers/media/v4l2-core/v4l2-subdev.c index 058c1a6..496f9bc 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -126,6 +126,55 @@ static int subdev_close(struct file *file)
> return 0;
> }
>
> +static int check_format(struct v4l2_subdev *sd,
> + struct v4l2_subdev_format *format)
> +{
> + if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> + format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return -EINVAL;
> +
> + if (format->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int check_crop(struct v4l2_subdev *sd, struct v4l2_subdev_crop
> *crop) +{
> + if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> + crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return -EINVAL;
> +
> + if (crop->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int check_selection(struct v4l2_subdev *sd,
> + struct v4l2_subdev_selection *sel)
> +{
> + if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
> + sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> + return -EINVAL;
> +
> + if (sel->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +static int check_edid(struct v4l2_subdev *sd, struct v4l2_subdev_edid
> *edid)
> +{
> + if (edid->pad >= sd->entity.num_pads)
> + return -EINVAL;
> +
> + if (edid->blocks && edid->edid == NULL)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg)
> {
> struct video_device *vdev = video_devdata(file);
> @@ -202,26 +251,20 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
> case VIDIOC_SUBDEV_G_FMT: {
> struct v4l2_subdev_format *format = arg;
> + int rval = check_format(sd, format);
How about declaring the variable once only at the beginning of the function ?
Apart from that,
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> - if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> - format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> -
> - if (format->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(sd, pad, get_fmt, subdev_fh, format);
> }
>
> case VIDIOC_SUBDEV_S_FMT: {
> struct v4l2_subdev_format *format = arg;
> + int rval = check_format(sd, format);
>
> - if (format->which != V4L2_SUBDEV_FORMAT_TRY &&
> - format->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> -
> - if (format->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(sd, pad, set_fmt, subdev_fh, format);
> }
> @@ -229,14 +272,10 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) case VIDIOC_SUBDEV_G_CROP: {
> struct v4l2_subdev_crop *crop = arg;
> struct v4l2_subdev_selection sel;
> - int rval;
> -
> - if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> - crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> + int rval = check_crop(sd, crop);
>
> - if (crop->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> rval = v4l2_subdev_call(sd, pad, get_crop, subdev_fh, crop);
> if (rval != -ENOIOCTLCMD)
> @@ -258,14 +297,10 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg) case VIDIOC_SUBDEV_S_CROP: {
> struct v4l2_subdev_crop *crop = arg;
> struct v4l2_subdev_selection sel;
> - int rval;
> -
> - if (crop->which != V4L2_SUBDEV_FORMAT_TRY &&
> - crop->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> + int rval = check_crop(sd, crop);
>
> - if (crop->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> rval = v4l2_subdev_call(sd, pad, set_crop, subdev_fh, crop);
> if (rval != -ENOIOCTLCMD)
> @@ -335,13 +370,10 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg)
>
> case VIDIOC_SUBDEV_G_SELECTION: {
> struct v4l2_subdev_selection *sel = arg;
> + int rval = check_selection(sd, sel);
>
> - if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
> - sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> -
> - if (sel->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(
> sd, pad, get_selection, subdev_fh, sel);
> @@ -349,13 +381,10 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg)
>
> case VIDIOC_SUBDEV_S_SELECTION: {
> struct v4l2_subdev_selection *sel = arg;
> + int rval = check_selection(sd, sel);
>
> - if (sel->which != V4L2_SUBDEV_FORMAT_TRY &&
> - sel->which != V4L2_SUBDEV_FORMAT_ACTIVE)
> - return -EINVAL;
> -
> - if (sel->pad >= sd->entity.num_pads)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(
> sd, pad, set_selection, subdev_fh, sel);
> @@ -363,22 +392,20 @@ static long subdev_do_ioctl(struct file *file,
> unsigned int cmd, void *arg)
>
> case VIDIOC_G_EDID: {
> struct v4l2_subdev_edid *edid = arg;
> + int rval = check_edid(sd, edid);
>
> - if (edid->pad >= sd->entity.num_pads)
> - return -EINVAL;
> - if (edid->blocks && edid->edid == NULL)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(sd, pad, get_edid, edid);
> }
>
> case VIDIOC_S_EDID: {
> struct v4l2_subdev_edid *edid = arg;
> + int rval = check_edid(sd, edid);
>
> - if (edid->pad >= sd->entity.num_pads)
> - return -EINVAL;
> - if (edid->blocks && edid->edid == NULL)
> - return -EINVAL;
> + if (rval)
> + return rval;
>
> return v4l2_subdev_call(sd, pad, set_edid, edid);
> }
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/1] v4l: subdev: Unify argument validation across IOCTLs
2014-06-03 17:16 ` Laurent Pinchart
@ 2014-06-04 11:17 ` Sakari Ailus
0 siblings, 0 replies; 3+ messages in thread
From: Sakari Ailus @ 2014-06-04 11:17 UTC (permalink / raw)
To: Laurent Pinchart; +Cc: linux-media
Hi Laurent,
Laurent Pinchart wrote:
> Hi Sakari,
>
> Thank you for the patch.
Thanks for the review!
> On Tuesday 03 June 2014 12:25:16 Sakari Ailus wrote:
...
>> @@ -202,26 +251,20 @@ static long subdev_do_ioctl(struct file *file,
>> unsigned int cmd, void *arg) #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API)
>> case VIDIOC_SUBDEV_G_FMT: {
>> struct v4l2_subdev_format *format = arg;
>> + int rval = check_format(sd, format);
>
> How about declaring the variable once only at the beginning of the function ?
I'll change that and resend.
--
Sakari Ailus
sakari.ailus@linux.intel.com
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-04 11:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-03 9:25 [PATCH 1/1] v4l: subdev: Unify argument validation across IOCTLs Sakari Ailus
2014-06-03 17:16 ` Laurent Pinchart
2014-06-04 11:17 ` Sakari Ailus
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.