Re: [PATCH v8 15/38] media: v4l: subdev: Return routes set using S_ROUTING

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

Thank you for the patch.

On Wed, Mar 13, 2024 at 09:24:53AM +0200, Sakari Ailus wrote:
> Return the routes set using S_ROUTING back to the user. Also reflect this
> in documentation.
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> ---
>  .../media/v4l/vidioc-subdev-g-routing.rst            |  5 +++--
>  drivers/media/v4l2-core/v4l2-subdev.c                | 12 +++++++++++-
>  2 files changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> index 8f9aa83275c5..6eb6a59570dc 100644
> --- a/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> +++ b/Documentation/userspace-api/media/v4l/vidioc-subdev-g-routing.rst
> @@ -43,8 +43,9 @@ The routing configuration determines the flows of data inside an entity.
>  Drivers report their current routing tables using the
>  ``VIDIOC_SUBDEV_G_ROUTING`` ioctl and application may enable or disable routes
>  with the ``VIDIOC_SUBDEV_S_ROUTING`` ioctl, by adding or removing routes and
> -setting or clearing flags of the  ``flags`` field of a
> -struct :c:type:`v4l2_subdev_route`.
> +setting or clearing flags of the ``flags`` field of a struct
> +:c:type:`v4l2_subdev_route`. Similarly to ``VIDIOC_SUBDEV_G_ROUTING``, also
> +``VIDIOC_SUBDEV_S_ROUTING`` returns the routes back to the user.
>  
>  All stream configurations are reset when ``VIDIOC_SUBDEV_S_ROUTING`` is
>  called. This means that the userspace must reconfigure all streams after calling
> diff --git a/drivers/media/v4l2-core/v4l2-subdev.c b/drivers/media/v4l2-core/v4l2-subdev.c
> index 486a5c3016de..a357ce318192 100644
> --- a/drivers/media/v4l2-core/v4l2-subdev.c
> +++ b/drivers/media/v4l2-core/v4l2-subdev.c
> @@ -957,8 +957,18 @@ static long subdev_do_ioctl(struct file *file, unsigned int cmd, void *arg,
>  		krouting.len_routes = routing->len_routes;
>  		krouting.routes = routes;
>  
> -		return v4l2_subdev_call(sd, pad, set_routing, state,
> +		rval = v4l2_subdev_call(sd, pad, set_routing, state,
>  					routing->which, &krouting);
> +		if (rval < 0)
> +			return rval;
> +
> +		memcpy((struct v4l2_subdev_route *)(uintptr_t)routing->routes,
> +		       state->routing.routes,
> +		       state->routing.num_routes *

I think this should be capped at len_routes.

> +		       sizeof(*state->routing.routes));
> +		routing->num_routes = state->routing.num_routes;
> +
> +		return 0;
>  	}
>  
>  	case VIDIOC_SUBDEV_G_ROUTING: {
> -- 
> 2.39.2
> 

-- 
Regards,

Laurent Pinchart