| Message ID | 20230914133323.198857-19-benjamin.gaignard@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DELETE_BUF ioctl | expand |
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@collabora.com> > --- > 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
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"); 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
vb2_get_buffer() already check if the requested index is valid. Stop duplicating this kind of check everywhere. Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> --- 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(-)