From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752095AbbJJPBK (ORCPT ); Sat, 10 Oct 2015 11:01:10 -0400 Received: from userp1040.oracle.com ([156.151.31.81]:27867 "EHLO userp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751472AbbJJPBI (ORCPT ); Sat, 10 Oct 2015 11:01:08 -0400 Date: Sat, 10 Oct 2015 18:00:10 +0300 From: Dan Carpenter To: Chen Feng Cc: gregkh@linuxfoundation.org, arve@android.com, riandrews@android.com, tranmanphong@gmail.com, mitchelh@codeaurora.org, tapaswenipathak@gmail.com, sumit.semwal@linaro.org, paul.gortmaker@windriver.com, gioh.kim@lge.com, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, yudongbin@hisilicon.com, saberlily.xia@hisilicon.com, suzhuangluan@hisilicon.com, kong.kongxinwei@hisilicon.com, xuyiping@hisilicon.com, z.liuxinliang@hisilicon.com, weidong2@hisilicon.com, w.f@huawei.com, qijiwen@hisilicon.com, peter.panshilin@hisilicon.com, dan.zhao@hisilicon.com, linuxarm@huawei.com Subject: Re: [PATCH V1 2/3] taging: android: ion: Add ion driver for Hi6220 SoC platform Message-ID: <20151010140013.GK7289@mwanda> References: <1444459703-183909-1-git-send-email-puck.chen@hisilicon.com> <1444459703-183909-2-git-send-email-puck.chen@hisilicon.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1444459703-183909-2-git-send-email-puck.chen@hisilicon.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Source-IP: aserv0021.oracle.com [141.146.126.233] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Oct 10, 2015 at 02:48:22PM +0800, Chen Feng wrote: > +static int hi6220_ion_probe(struct platform_device *pdev) > +{ > + int i; > + int err = 0; > + static struct ion_platform_heap *p_heap; > + > + idev = ion_device_create(NULL); > + hi6220_set_platform_data(pdev); > + heaps = devm_kzalloc(&pdev->dev, > + sizeof(struct ion_heap *) * num_heaps, > + GFP_KERNEL); > + if (!heaps) > + return -ENOMEM; > + > + /* > + * create the heaps as specified in the dts file > + */ > + for (i = 0; i < num_heaps; i++) { > + p_heap = heaps_data[i]; > + heaps[i] = ion_heap_create(p_heap); > + if (IS_ERR_OR_NULL(heaps[i])) { > + err = PTR_ERR(heaps[i]); > + goto err_free_heaps; > + } > + > + ion_device_add_heap(idev, heaps[i]); > + > + pr_info("%s: adding heap %s of type %d with %lx@%lx\n", > + __func__, p_heap->name, p_heap->type, > + p_heap->base, (unsigned long)p_heap->size); > + } > + return err; > + > +err_free_heaps: > + ion_device_destroy(idev); > + for (i = 0; i < num_heaps; ++i) { > + ion_heap_destroy(heaps[i]); > + heaps[i] = NULL; > + } > + > + return err; > +} Thanks this is better but still not quite right. You have to unwind in the reverse order from how you allocated things. err_free_heaps: for (i = 0; i < num_heaps; ++i) { ion_heap_destroy(heaps[i]); heaps[i] = NULL; } err_destroy_idev: ion_device_destroy(idev); return err; And earlier it should be: idev = ion_device_create(NULL); hi6220_set_platform_data(pdev); heaps = devm_kzalloc(&pdev->dev, sizeof(struct ion_heap *) * num_heaps, GFP_KERNEL); - if (!heaps) - return -ENOMEM; + if (!heaps) { + err = -ENOMEM; + goto err_destroy_idev; + } Otherwise we leak some resources if we can't allocate "heaps". regards, dan carpenter