Re: [PATCH 1/2] v4l2-ioctl: simplify code

[Niklas Söderlund]

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

[Hans Verkuil]

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

[Niklas Söderlund]

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, );
> - if (ret)
> - return ret;
> - p->bounds = s.r;
> + ret = ops->vidioc_g_selection(file, fh, );
> + 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, );
> - if (ret)
> - return ret;
> - p->defrect = s.r;
> - }
> + ret = ops->vidioc_g_selection(file, fh, );
> + 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