diff mbox

media: venus: fix duplicated code for different branches

Message ID 20170817231234.GA6674@embeddedgus (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo A. R. Silva Aug. 17, 2017, 11:12 p.m. UTC
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 <gustavo@embeddedor.com>
---
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(-)

Comments

Hans Verkuil Aug. 18, 2017, 6:57 a.m. UTC | #1
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

On 08/18/2017 01:12 AM, Gustavo A. R. Silva wrote:
> 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 <gustavo@embeddedor.com>
> ---
> 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(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 5f4434c..8a5c467 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -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 Aug. 18, 2017, 7:52 a.m. UTC | #2
Hi Gustavo,

On 08/18/2017 02:12 AM, Gustavo A. R. Silva wrote:
> 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 <gustavo@embeddedor.com>
> ---
> 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.

> 
>  drivers/media/platform/qcom/venus/helpers.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 5f4434c..8a5c467 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -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_*.

> -
> +	v4l2_m2m_src_buf_remove_by_buf(m2m_ctx, vbuf);
>  	v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_ERROR);
>  }
>  
>
Gustavo A. R. Silva Aug. 18, 2017, 3:55 p.m. UTC | #3
Hi Stanimir,

On 08/18/2017 02:52 AM, Stanimir Varbanov wrote:
> Hi Gustavo,
>
> On 08/18/2017 02:12 AM, Gustavo A. R. Silva wrote:
>> 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 <gustavo@embeddedor.com>
>> ---
>> 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.

>>
>>  drivers/media/platform/qcom/venus/helpers.c | 6 +-----
>>  1 file changed, 1 insertion(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
>> index 5f4434c..8a5c467 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.c
>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>> @@ -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_*.
>

I'll send a patch to fix this bug shortly

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

Thanks!
diff mbox

Patch

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 5f4434c..8a5c467 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -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);
 }