Re: [PATCH 1/2] v4l2-ioctl: simplify code
On 2016-03-14 13:52:17 +0100, Hans Verkuil wrote:
> On 03/14/2016 01:42 PM, Niklas Söderlund wrote:
> > Hi Hans,
> >
> > On 2016-02-29 11:16:39 +0100, Hans Verkuil wrote:
> >> From: Hans Verkuil
> >>
> >> Instead of a big if at the beginning, just check if g_selection == NULL
> >> and call the cropcap op immediately and return the result.
> >>
> >> No functional changes in this patch.
> >>
> >> Signed-off-by: Hans Verkuil
> >> ---
> >> drivers/media/v4l2-core/v4l2-ioctl.c | 44
> >> ++--
> >> 1 file changed, 22 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> >> b/drivers/media/v4l2-core/v4l2-ioctl.c
> >> index 86c4c19..67dbb03 100644
> >> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> >> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> >> @@ -2157,33 +2157,33 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops
> >> *ops,
> >>struct file *file, void *fh, void *arg)
> >> {
> >>struct v4l2_cropcap *p = arg;
> >> + struct v4l2_selection s = { .type = p->type };
> >> + int ret;
> >>
> >> - if (ops->vidioc_g_selection) {
> >> - struct v4l2_selection s = { .type = p->type };
> >> - int ret;
> >> + if (ops->vidioc_g_selection == NULL)
> >> + return ops->vidioc_cropcap(file, fh, p);
> >
> > I might be missing something but is there a guarantee
> > ops->vidioc_cropcap is not NULL here?
>
> There is, either vidioc_g_selection or vidioc_cropcap will always be
> non-NULL. Since g_selection == NULL it follows that cropcap != NULL.
>
> But I admit that it isn't exactly obvious since the test that ensures
> this is in determine_valid_ioctls() in v4l2-dev.c.
Nice, thanks for clarifying.
Reviewed-by: Niklas Söderlund
--
Regards,
Niklas Söderlund
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] v4l2-ioctl: simplify code
On 03/14/2016 01:42 PM, Niklas Söderlund wrote:
> Hi Hans,
>
> On 2016-02-29 11:16:39 +0100, Hans Verkuil wrote:
>> From: Hans Verkuil
>>
>> Instead of a big if at the beginning, just check if g_selection == NULL
>> and call the cropcap op immediately and return the result.
>>
>> No functional changes in this patch.
>>
>> Signed-off-by: Hans Verkuil
>> ---
>> drivers/media/v4l2-core/v4l2-ioctl.c | 44
>> ++--
>> 1 file changed, 22 insertions(+), 22 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
>> b/drivers/media/v4l2-core/v4l2-ioctl.c
>> index 86c4c19..67dbb03 100644
>> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
>> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
>> @@ -2157,33 +2157,33 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops
>> *ops,
>> struct file *file, void *fh, void *arg)
>> {
>> struct v4l2_cropcap *p = arg;
>> +struct v4l2_selection s = { .type = p->type };
>> +int ret;
>>
>> -if (ops->vidioc_g_selection) {
>> -struct v4l2_selection s = { .type = p->type };
>> -int ret;
>> +if (ops->vidioc_g_selection == NULL)
>> +return ops->vidioc_cropcap(file, fh, p);
>
> I might be missing something but is there a guarantee
> ops->vidioc_cropcap is not NULL here?
There is, either vidioc_g_selection or vidioc_cropcap will always be
non-NULL. Since g_selection == NULL it follows that cropcap != NULL.
But I admit that it isn't exactly obvious since the test that ensures
this is in determine_valid_ioctls() in v4l2-dev.c.
Regards,
Hans
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] v4l2-ioctl: simplify code
Hi Hans,
On 2016-02-29 11:16:39 +0100, Hans Verkuil wrote:
> From: Hans Verkuil
>
> Instead of a big if at the beginning, just check if g_selection == NULL
> and call the cropcap op immediately and return the result.
>
> No functional changes in this patch.
>
> Signed-off-by: Hans Verkuil
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 44
> ++--
> 1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 86c4c19..67dbb03 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -2157,33 +2157,33 @@ static int v4l_cropcap(const struct v4l2_ioctl_ops
> *ops,
> struct file *file, void *fh, void *arg)
> {
> struct v4l2_cropcap *p = arg;
> + struct v4l2_selection s = { .type = p->type };
> + int ret;
>
> - if (ops->vidioc_g_selection) {
> - struct v4l2_selection s = { .type = p->type };
> - int ret;
> + if (ops->vidioc_g_selection == NULL)
> + return ops->vidioc_cropcap(file, fh, p);
I might be missing something but is there a guarantee
ops->vidioc_cropcap is not NULL here?
>
> - /* obtaining bounds */
> - if (V4L2_TYPE_IS_OUTPUT(p->type))
> - s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
> - else
> - s.target = V4L2_SEL_TGT_CROP_BOUNDS;
> + /* obtaining bounds */
> + if (V4L2_TYPE_IS_OUTPUT(p->type))
> + s.target = V4L2_SEL_TGT_COMPOSE_BOUNDS;
> + else
> + s.target = V4L2_SEL_TGT_CROP_BOUNDS;
>
> - ret = ops->vidioc_g_selection(file, fh, &s);
> - if (ret)
> - return ret;
> - p->bounds = s.r;
> + ret = ops->vidioc_g_selection(file, fh, &s);
> + if (ret)
> + return ret;
> + p->bounds = s.r;
>
> - /* obtaining defrect */
> - if (V4L2_TYPE_IS_OUTPUT(p->type))
> - s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
> - else
> - s.target = V4L2_SEL_TGT_CROP_DEFAULT;
> + /* obtaining defrect */
> + if (V4L2_TYPE_IS_OUTPUT(p->type))
> + s.target = V4L2_SEL_TGT_COMPOSE_DEFAULT;
> + else
> + s.target = V4L2_SEL_TGT_CROP_DEFAULT;
>
> - ret = ops->vidioc_g_selection(file, fh, &s);
> - if (ret)
> - return ret;
> - p->defrect = s.r;
> - }
> + ret = ops->vidioc_g_selection(file, fh, &s);
> + if (ret)
> + return ret;
> + p->defrect = s.r;
>
> /* setting trivial pixelaspect */
> p->pixelaspect.numerator = 1;
> --
> 2.7.0
>
--
Regards,
Niklas Söderlund
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
