Re: [PATCH v7 09/49] media: atomisp: Use vb2_get_buffer() instead of directly access to buffers array

Hans Verkuil
2023-09-19T09:41:46Z

On 14/09/2023 15:32, Benjamin Gaignard wrote:
> Use vb2_get_buffer() instead of directly access to vb2_buffer buffer array.
> This could allow to change the type bufs[] field of vb2_buffer structure if
> needed.
> After each call to vb2_get_buffer() we need to be sure that we get
> a valid pointer so check the return value of all of them.

This last paragraph does not apply to this specific patch since we know
here that the buffer will always be valid.

Perhaps change this to something along the lines of:

"No need to check the result of vb2_get_buffer, vb2_ioctl_dqbuf() already
checked that it is valid."

Regards,

Hans

>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com>
> ---
> drivers/staging/media/atomisp/pci/atomisp_ioctl.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> index d2174156573a..4b65c69fa60d 100644
> --- a/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> +++ b/drivers/staging/media/atomisp/pci/atomisp_ioctl.c
> @@ -1061,7 +1061,7 @@ static int atomisp_dqbuf_wrapper(struct file *file, void *fh, struct v4l2_buffer
> if (ret)
> return ret;
>
> - vb = pipe->vb_queue.bufs[buf->index];
> + vb = vb2_get_buffer(&pipe->vb_queue, buf->index);
> frame = vb_to_frame(vb);
>
> buf->reserved = asd->frame_status[buf->index];