Skip to contentSkip to conversation

vb2: check for sane values from queue_setup

PATCH Superseded Johan Fjeldtvedt 2 participants 2 messages Sep 11, 2018

Subsystems (2)
Media (V4L/DVB) VIDEOBUF2 framework

v2 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Johan Fjeldtvedt

author
#0 lore raw

PATCH v2 1/1 vb2: check for sane values from queue_setup

Warn when driver sets 0 number of planes or 0 as plane sizes.

Signed-off-by Johan Fjeldtvedt <[email protected]>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/media/common/videobuf2/videobuf2-core.c +7 -0
index f32ec7342ef0..6f903740d813 100644
@@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	unsigned int num_buffers, allocated_buffers, num_planes = 0;
 	unsigned plane_sizes[VB2_MAX_PLANES] = { };
 	int ret;
+	int i;
 
 	if (q->streaming) {
 		dprintk(1, "streaming active\n");

@@ -718,6 +719,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	if (ret)
 		return ret;
 
+	/* Check that driver has set sane values */
+	WARN_ON(!num_buffers);
+
+	for (i = 0; i < num_buffers; i++)
+		WARN_ON(!plane_sizes[i]);
+
 	/* Finally, allocate buffers and video memory */
 	allocated_buffers =
 		__vb2_queue_alloc(q, memory, num_buffers, num_planes, plane_sizes);
Signature · 1 line
2.17.1
#1 lore raw

A few more comments:

Quoted 1 line · Johan Fjeldtvedt

Warn when driver sets 0 number of planes or 0 as plane sizes.

It should also return an error, since there is no point continuing with garbage values.

Also add *why* this is useful to the commit. I know why, since I suggested it to you, but others don't :-)

Quoted 19 lines
Signed-off-by Johan Fjeldtvedt <[email protected]>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 7 +++++++
 1 file changed, 7 insertions(+)
drivers/media/common/videobuf2/videobuf2-core.c +1 -0
index f32ec7342ef0..6f903740d813 100644
@@ -662,6 +662,7 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	unsigned int num_buffers, allocated_buffers, num_planes = 0;
 	unsigned plane_sizes[VB2_MAX_PLANES] = { };
 	int ret;
+	int i;
 
 	if (q->streaming) {
 		dprintk(1, "streaming active\n");

drivers/media/common/videobuf2/videobuf2-core.c int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, · line 723

Quoted 5 lines
drivers/media/common/videobuf2/videobuf2-core.c +2 -0
@@ -718,6 +719,12 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory,
 	if (ret)
 		return ret;
 
+	/* Check that driver has set sane values */
+	WARN_ON(!num_buffers);

Just return an error here. EINVAL is not unreasonable. So:

	if (WARN_ON(!num_buffers))
		return -EINVAL;

drivers/media/common/videobuf2/videobuf2-core.c int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, · line 726

Quoted 3 lines
drivers/media/common/videobuf2/videobuf2-core.c +3 -0
+
+	for (i = 0; i < num_buffers; i++)
+		WARN_ON(!plane_sizes[i]);

Ditto.

drivers/media/common/videobuf2/videobuf2-core.c int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, · line 730

Quoted 5 lines
drivers/media/common/videobuf2/videobuf2-core.c +1 -0
+
 	/* Finally, allocate buffers and video memory */
 	allocated_buffers =
 		__vb2_queue_alloc(q, memory, num_buffers, num_planes, plane_sizes);

Regards,

Hans