[PATCH][next] dma-buf: heaps: remove redundant assignment to variable ret

[PATCH][next] dma-buf: heaps: remove redundant assignment to variable ret

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The variable ret is being assigned with a value that is never
read, it is being re-assigned the same value on the err0 exit
path. The assignment is redundant and hence can be removed.

Addresses-Coverity: ("Unused value")
Fixes: 47a32f9c1226 ("dma-buf: heaps: Add system heap to dmabuf heaps")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/dma-buf/heaps/system_heap.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
index 455782efbb32..817a1667bd57 100644
--- a/drivers/dma-buf/heaps/system_heap.c
+++ b/drivers/dma-buf/heaps/system_heap.c
@@ -55,10 +55,8 @@ static int system_heap_allocate(struct dma_heap *heap,
 	helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
 					     sizeof(*helper_buffer->pages),
 					     GFP_KERNEL);
-	if (!helper_buffer->pages) {
-		ret = -ENOMEM;
+	if (!helper_buffer->pages)
 		goto err0;
-	}
 
 	for (pg = 0; pg < helper_buffer->pagecount; pg++) {
 		/*
-- 
2.20.1

Re: [PATCH][next] dma-buf: heaps: remove redundant assignment to variable ret

[Andrew F. Davis]

On 10/30/19 11:02 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable ret is being assigned with a value that is never
> read, it is being re-assigned the same value on the err0 exit
> path. The assignment is redundant and hence can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Fixes: 47a32f9c1226 ("dma-buf: heaps: Add system heap to dmabuf heaps")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---


The root of the issue is that ret is not used in the error path, it
should be, I suggest this fix:

> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -98,7 +98,7 @@ static int system_heap_allocate(struct dma_heap *heap,
>  err0:
>         kfree(helper_buffer);
>  
> -       return -ENOMEM;
> +       return ret;
>  }
>  
>  static const struct dma_heap_ops system_heap_ops = {

Andrew


>  drivers/dma-buf/heaps/system_heap.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/dma-buf/heaps/system_heap.c b/drivers/dma-buf/heaps/system_heap.c
> index 455782efbb32..817a1667bd57 100644
> --- a/drivers/dma-buf/heaps/system_heap.c
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -55,10 +55,8 @@ static int system_heap_allocate(struct dma_heap *heap,
>  	helper_buffer->pages = kmalloc_array(helper_buffer->pagecount,
>  					     sizeof(*helper_buffer->pages),
>  					     GFP_KERNEL);
> -	if (!helper_buffer->pages) {
> -		ret = -ENOMEM;
> +	if (!helper_buffer->pages)
>  		goto err0;
> -	}
>  
>  	for (pg = 0; pg < helper_buffer->pagecount; pg++) {
>  		/*
> 

Re: [PATCH][next] dma-buf: heaps: remove redundant assignment to variable ret

[John Stultz]

On Wed, Oct 30, 2019 at 8:45 AM Andrew F. Davis <afd@ti.com> wrote:
>
> On 10/30/19 11:02 AM, Colin King wrote:
> > From: Colin Ian King <colin.king@canonical.com>
> >
> > The variable ret is being assigned with a value that is never
> > read, it is being re-assigned the same value on the err0 exit
> > path. The assignment is redundant and hence can be removed.
> >
> > Addresses-Coverity: ("Unused value")
> > Fixes: 47a32f9c1226 ("dma-buf: heaps: Add system heap to dmabuf heaps")
> > Signed-off-by: Colin Ian King <colin.king@canonical.com>
> > ---
>
>
> The root of the issue is that ret is not used in the error path, it
> should be, I suggest this fix:
>
> > --- a/drivers/dma-buf/heaps/system_heap.c
> > +++ b/drivers/dma-buf/heaps/system_heap.c
> > @@ -98,7 +98,7 @@ static int system_heap_allocate(struct dma_heap *heap,
> >  err0:
> >         kfree(helper_buffer);
> >
> > -       return -ENOMEM;
> > +       return ret;
> >  }

Sounds good! If its ok I'll generate a commit crediting Colin for
reporting the issue and Andrew for the fix and submit it to Sumit.

Many thanks to you both!
-john

Re: [PATCH][next] dma-buf: heaps: remove redundant assignment to variable ret

[Colin Ian King]

On 30/10/2019 17:21, John Stultz wrote:
> On Wed, Oct 30, 2019 at 8:45 AM Andrew F. Davis <afd@ti.com> wrote:
>>
>> On 10/30/19 11:02 AM, Colin King wrote:
>>> From: Colin Ian King <colin.king@canonical.com>
>>>
>>> The variable ret is being assigned with a value that is never
>>> read, it is being re-assigned the same value on the err0 exit
>>> path. The assignment is redundant and hence can be removed.
>>>
>>> Addresses-Coverity: ("Unused value")
>>> Fixes: 47a32f9c1226 ("dma-buf: heaps: Add system heap to dmabuf heaps")
>>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>>> ---
>>
>>
>> The root of the issue is that ret is not used in the error path, it
>> should be, I suggest this fix:
>>
>>> --- a/drivers/dma-buf/heaps/system_heap.c
>>> +++ b/drivers/dma-buf/heaps/system_heap.c
>>> @@ -98,7 +98,7 @@ static int system_heap_allocate(struct dma_heap *heap,
>>>  err0:
>>>         kfree(helper_buffer);
>>>
>>> -       return -ENOMEM;
>>> +       return ret;
>>>  }
> 
> Sounds good! If its ok I'll generate a commit crediting Colin for
> reporting the issue and Andrew for the fix and submit it to Sumit.

Thanks for the correct fix.

Colin

> 
> Many thanks to you both!
> -john
>