| Message ID | 20231031163104.112469-15-benjamin.gaignard@collabora.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add DELETE_BUF ioctl | expand |
Hi Benjamin, W dniu 31.10.2023 o 17:30, Benjamin Gaignard pisze: > Use vb2_get_buffer() instead of direct access to the vb2_queue bufs array. > This allows us to change the type of the bufs in the future. > 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. > Remove index range test since it is done by vb2_get_buffer(). Actually, the patch uses vb2_get_buffer() instead of using vb2_get_buffer(). IOW vb2_get_buffer() continues to be used before and after this patch is applied. I'd rather reformulate the commit message body to say that we remove index check because it is already performed by vb2_get_buffer(), but introduce a check for a NULL result. Regards, Andrzej > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> > CC: Jean-Christophe Trotin <jean-christophe.trotin@foss.st.com> > --- > drivers/media/platform/st/sti/hva/hva-v4l2.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/st/sti/hva/hva-v4l2.c b/drivers/media/platform/st/sti/hva/hva-v4l2.c > index 3a848ca32a0e..cfe83e9dc01b 100644 > --- a/drivers/media/platform/st/sti/hva/hva-v4l2.c > +++ b/drivers/media/platform/st/sti/hva/hva-v4l2.c > @@ -569,14 +569,11 @@ static int hva_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) > struct vb2_buffer *vb2_buf; > > vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, buf->type); > - > - if (buf->index >= vq->num_buffers) { > - dev_dbg(dev, "%s buffer index %d out of range (%d)\n", > - ctx->name, buf->index, vq->num_buffers); > + vb2_buf = vb2_get_buffer(vq, buf->index); > + if (!vb2_buf) { > + dev_dbg(dev, "%s buffer index %d not found\n", ctx->name, buf->index); > return -EINVAL; > } > - > - vb2_buf = vb2_get_buffer(vq, buf->index); > stream = to_hva_stream(to_vb2_v4l2_buffer(vb2_buf)); > stream->bytesused = buf->bytesused; > }
diff --git a/drivers/media/platform/st/sti/hva/hva-v4l2.c b/drivers/media/platform/st/sti/hva/hva-v4l2.c index 3a848ca32a0e..cfe83e9dc01b 100644 --- a/drivers/media/platform/st/sti/hva/hva-v4l2.c +++ b/drivers/media/platform/st/sti/hva/hva-v4l2.c @@ -569,14 +569,11 @@ static int hva_qbuf(struct file *file, void *priv, struct v4l2_buffer *buf) struct vb2_buffer *vb2_buf; vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, buf->type); - - if (buf->index >= vq->num_buffers) { - dev_dbg(dev, "%s buffer index %d out of range (%d)\n", - ctx->name, buf->index, vq->num_buffers); + vb2_buf = vb2_get_buffer(vq, buf->index); + if (!vb2_buf) { + dev_dbg(dev, "%s buffer index %d not found\n", ctx->name, buf->index); return -EINVAL; } - - vb2_buf = vb2_get_buffer(vq, buf->index); stream = to_hva_stream(to_vb2_v4l2_buffer(vb2_buf)); stream->bytesused = buf->bytesused; }
Use vb2_get_buffer() instead of direct access to the vb2_queue bufs array. This allows us to change the type of the bufs in the future. 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. Remove index range test since it is done by vb2_get_buffer(). Signed-off-by: Benjamin Gaignard <benjamin.gaignard@collabora.com> CC: Jean-Christophe Trotin <jean-christophe.trotin@foss.st.com> --- drivers/media/platform/st/sti/hva/hva-v4l2.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)