media: venus: fix duplicated code for different branches

PATCH In review Gustavo A. R. Silva 4 participants 9 messages Aug 17, 2017 – Aug 23, 2017

Subsystems (4)

Media (V4L/DVB) Arm Qualcomm mailing list Qualcomm media platform Qualcomm venus video accelerator driver

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-08-17 23:12 UTC #0 lore raw

PATCH 1/1 media: venus: fix duplicated code for different branches

Refactor code in order to avoid identical code for different branches.

This issue was detected with the help of Coccinelle.

Addresses-Coverity-ID: 1415317

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
This code was reported by Coverity and it was tested by compilation only. Please, verify if this is an actual bug.

 drivers/media/platform/qcom/venus/helpers.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/media/platform/qcom/venus/helpers.c +1 -5

index 5f4434c..8a5c467 100644
@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
 	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
-	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-
+	v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }

Signature · 1 line

2.5.0

Hans Verkuil

2017-08-18 06:57 UTC #1 lore raw

Stanimir, please review this! I suspect that this is the wrong fix and that the first v4l2_m2m_src_buf_remove_by_buf should be v4l2_m2m_dst_buf_remove_by_buf instead.

Regards,

Hans

Quoted 31 lines · Gustavo A. R. Silva

Refactor code in order to avoid identical code for different branches.
This issue was detected with the help of Coccinelle.
Addresses-Coverity-ID: 1415317
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
This code was reported by Coverity and it was tested by compilation only. Please, verify if this is an actual bug.
 drivers/media/platform/qcom/venus/helpers.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/media/platform/qcom/venus/helpers.c +1 -5
index 5f4434c..8a5c467 100644
@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
 	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
-	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-
+	v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }
 

Stanimir Varbanov

2017-08-18 07:52 UTC #2 lore raw

In reply to Gustavo A. R. Silva

Hi Gustavo,

Quoted 9 lines · Gustavo A. R. Silva

Refactor code in order to avoid identical code for different branches.
This issue was detected with the help of Coccinelle.
Addresses-Coverity-ID: 1415317
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
This code was reported by Coverity and it was tested by compilation only. Please, verify if this is an actual bug.

Yes looks like copy/paste error, and yes it is a bug.

Quoted 9 lines

 drivers/media/platform/qcom/venus/helpers.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/media/platform/qcom/venus/helpers.c +0 -0

index 5f4434c..8a5c467 100644

drivers/media/platform/qcom/venus/helpers.c static void return_buf_error(struct venus_inst *inst, · line 246

Quoted 7 lines

drivers/media/platform/qcom/venus/helpers.c +0 -4

@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
 	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
-	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);

the correct fix must replace the second v4l2_m2m_src_* with v4l2_m2m_dst_*.

Quoted 6 lines

drivers/media/platform/qcom/venus/helpers.c +1 -1

-
+	v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }

Signature · 2 lines

regards,
Stan

Gustavo A. R. Silva

author

2017-08-18 15:55 UTC #3 lore raw

Hi Stanimir,

Quoted 7 lines · Stanimir Varbanov

Hi Gustavo,
Quoted 4 lines · Gustavo A. R. Silva
Refactor code in order to avoid identical code for different branches.
This issue was detected with the help of Coccinelle.

Quoted 8 lines

Quoted 5 lines
Addresses-Coverity-ID: 1415317
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
This code was reported by Coverity and it was tested by compilation only. Please, verify if this is an actual bug.
Yes looks like copy/paste error, and yes it is a bug.

Thank you for reviewing it.

Quoted 12 lines

 drivers/media/platform/qcom/venus/helpers.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/media/platform/qcom/venus/helpers.c +0 -0

index 5f4434c..8a5c467 100644
@@ -240,11 +240,7 @@ static void return_buf_error(struct venus_inst *inst,
 {
 	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;

Quoted 7 lines

drivers/media/platform/qcom/venus/helpers.c static void return_buf_error(struct venus_inst *inst, · line 246
Quoted 4 lines
drivers/media/platform/qcom/venus/helpers.c +0 -4
-	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
-	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
the correct fix must replace the second v4l2_m2m_src_* with v4l2_m2m_dst_*.

I'll send a patch to fix this bug shortly

Quoted 7 lines

drivers/media/platform/qcom/venus/helpers.c static void return_buf_error(struct venus_inst *inst, · line 246
Quoted 6 lines
drivers/media/platform/qcom/venus/helpers.c +1 -1
-
+	v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }
 

Thanks!

Signature · 1 line

Gustavo A. R. Silva

Gustavo A. R. Silva

author

2017-08-18 16:07 UTC #4 lore raw

PATCH v2 1/1 venus: fix copy/paste error in return_buf_error

Call function v4l2_m2m_dst_buf_remove_by_buf() instead of v4l2_m2m_src_buf_remove_by_buf()

Addresses-Coverity-ID: 1415317

Cc Stanimir Varbanov <[email protected]>

Cc Hans Verkuil <[email protected]>

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
Stanimir Varbanov confirmed this is a bug. The correct fix is to call function v4l2_m2m_dst_buf_remove_by_buf instead of function v4l2_m2m_src_buf_remove_by_buf in the _else_ branch.

 drivers/media/platform/qcom/venus/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/platform/qcom/venus/helpers.c +1 -1

index 5f4434c..2d61879 100644
@@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst,
 	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
+		v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf);
 
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }

Signature · 1 line

2.5.0

Stanimir Varbanov

Acked-by on v2 1/1

2017-08-21 09:14 UTC #5 lore raw

Re: [PATCH v2] venus: fix copy/paste error in return_buf_error

Thanks Gustavo!

Quoted 7 lines · Gustavo A. R. Silva

Call function v4l2_m2m_dst_buf_remove_by_buf() instead of v4l2_m2m_src_buf_remove_by_buf()
Addresses-Coverity-ID: 1415317
Cc Stanimir Varbanov <[email protected]>
Cc Hans Verkuil <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>

Quoted 8 lines

---
Changes in v2:
Stanimir Varbanov confirmed this is a bug. The correct fix is to call function v4l2_m2m_dst_buf_remove_by_buf instead of function v4l2_m2m_src_buf_remove_by_buf in the _else_ branch.
 drivers/media/platform/qcom/venus/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Acked-by Stanimir Varbanov <[email protected]>

Quoted 15 lines

drivers/media/platform/qcom/venus/helpers.c +1 -1

index 5f4434c..2d61879 100644
@@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst,
 	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
+		v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf);
 
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }

Signature · 2 lines

regards,
Stan

Gustavo A. R. Silva

author

2017-08-22 19:22 UTC #6 lore raw

Re: [PATCH v2] venus: fix copy/paste error in return_buf_error

Quoted 2 lines · Stanimir Varbanov

Thanks Gustavo!

Glad to help. :)

Quoted 35 lines · Gustavo A. R. Silva

Quoted 7 lines
Call function v4l2_m2m_dst_buf_remove_by_buf() instead of v4l2_m2m_src_buf_remove_by_buf()
Addresses-Coverity-ID: 1415317
Cc Stanimir Varbanov <[email protected]>
Cc Hans Verkuil <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Quoted 8 lines
---
Changes in v2:
Stanimir Varbanov confirmed this is a bug. The correct fix is to call function v4l2_m2m_dst_buf_remove_by_buf instead of function v4l2_m2m_src_buf_remove_by_buf in the _else_ branch.
 drivers/media/platform/qcom/venus/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by Stanimir Varbanov <[email protected]>
Quoted 15 lines
drivers/media/platform/qcom/venus/helpers.c +1 -1
index 5f4434c..2d61879 100644
@@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst,
 	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
+		v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf);
 
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }

Signature · 1 line

Gustavo A. R. Silva

Stanimir Varbanov

2017-08-23 12:59 UTC #7 lore raw

Re: [PATCH v2] venus: fix copy/paste error in return_buf_error

In reply to Gustavo A. R. Silva

Gustavo,

could you resend the patch with the Acked-by and Cc stable kernel.

Quoted 30 lines · Gustavo A. R. Silva

Call function v4l2_m2m_dst_buf_remove_by_buf() instead of v4l2_m2m_src_buf_remove_by_buf()
Addresses-Coverity-ID: 1415317
Cc Stanimir Varbanov <[email protected]>
Cc Hans Verkuil <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
Changes in v2:
Stanimir Varbanov confirmed this is a bug. The correct fix is to call function v4l2_m2m_dst_buf_remove_by_buf instead of function v4l2_m2m_src_buf_remove_by_buf in the _else_ branch.
 drivers/media/platform/qcom/venus/helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/media/platform/qcom/venus/helpers.c +1 -1
index 5f4434c..2d61879 100644
@@ -243,7 +243,7 @@ static void return_buf_error(struct venus_inst *inst,
 	if (vbuf->vb2_buf.type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
 	else
-		v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
+		v4l2_m2m_dst_buf_remove_by_buf(m2m_ctx, vbuf);
 
 	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
 }

Signature · 2 lines

regards,
Stan

Stanimir Varbanov

2017-08-23 15:14 UTC #8 lore raw

Re: [PATCH v2] venus: fix copy/paste error in return_buf_error

Quoted 3 lines · Stanimir Varbanov

Gustavo,
could you resend the patch with the Acked-by and Cc stable kernel.

No need to resend, sorry for the noise.

Signature · 2 lines

regards,
Stan