| Message ID | 20191030150253.10596-1-colin.king@canonical.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [next] dma-buf: heaps: remove redundant assignment to variable ret | expand |
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++) { > /* >
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
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 >
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++) { /*