On 14/09/2023 15:32, Benjamin Gaignard wrote:
> vb2_get_buffer() already check if the requested index is valid.
typo: check -> checks
> Stop duplicating this kind of check everywhere.
Also mention you moved it from the header to videobuf2-core.c.
Although I am not sure if it belongs in this patch, it is not
needed for this. I think it is better to move it either into a
separate patch, or move it to the patch where it is really needed.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx>
> ---
> drivers/media/common/videobuf2/videobuf2-core.c | 8 ++++++++
> drivers/media/common/videobuf2/videobuf2-v4l2.c | 13 -------------
> include/media/videobuf2-core.h | 8 +-------
> 3 files changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index ee4df7c68397..2add7a6795e7 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -660,6 +660,14 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
> }
> }
>
> +struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q, unsigned int index)
> +{
> + if (index < q->num_buffers)
> + return q->bufs[index];
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(vb2_get_buffer);
> +
> bool vb2_buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb)
> {
> unsigned int plane;
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 87c2d5916960..f10b70d8e66a 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -378,11 +378,6 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md
> return -EINVAL;
> }
>
> - if (b->index >= q->num_buffers) {
> - dprintk(q, 1, "%s: buffer index out of range\n", opname);
> - return -EINVAL;
> - }
> -
> vb = vb2_get_buffer(q, b->index);
> if (!vb) {
> dprintk(q, 1, "%s: buffer is NULL\n", opname);
> @@ -829,10 +824,6 @@ int vb2_qbuf(struct vb2_queue *q, struct media_device *mdev,
> return -EBUSY;
> }
>
> - if (b->index >= q->num_buffers) {
> - dprintk(q, 1, "buffer index out of range\n");
> - return -EINVAL;
> - }
> vb = vb2_get_buffer(q, b->index);
> if (!vb) {
> dprintk(q, 1, "can't find the requested buffer\n");
> @@ -904,10 +895,6 @@ int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
> {
> struct vb2_buffer *vb;
>
> - if (eb->index >= q->num_buffers) {
> - dprintk(q, 1, "buffer index out of range\n");
> - return -EINVAL;
> - }
> vb = vb2_get_buffer(q, eb->index);
> if (!vb) {
> dprintk(q, 1, "can't find the requested buffer\n");
This patch should be folded into 11/49. It is 11/49 that introduced these
duplicate messages, and it should have removed those directly.
Regards,
Hans
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 97153c69583f..25ca395616a7 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -1238,13 +1238,7 @@ static inline void vb2_clear_last_buffer_dequeued(struct vb2_queue *q)
> * operation, so the buffer lifetime should be taken into
> * consideration.
> */
> -static inline struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q,
> - unsigned int index)
> -{
> - if (index < q->num_buffers)
> - return q->bufs[index];
> - return NULL;
> -}
> +struct vb2_buffer *vb2_get_buffer(struct vb2_queue *q, unsigned int index);
>
> /*
> * The following functions are not part of the vb2 core API, but are useful
