From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f48.google.com (mail-pa0-f48.google.com [209.85.220.48]) by dpdk.org (Postfix) with ESMTP id 2A3E4C770 for ; Fri, 26 Jun 2015 03:30:48 +0200 (CEST) Received: by pabvl15 with SMTP id vl15so59439988pab.1 for ; Thu, 25 Jun 2015 18:30:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=RxTTp7I+omKpo8ADpYiYLp4c/DglSPj4qkFk53KAHNM=; b=dNCIjGAjup5h0uF4tUShOSCLfNol89ctbMftJsSFhwJpN4YZwR0V9iWuCuubgETK1I CMhy/0ihjXOfr+Anti8NVlojKuGrFXi3rH0Cp0V8bi8eL9d9bgoSbxz9aQNve+iM08Dp unBJLARshTdiN/C9YXzoKhyctTbJOXB7sHWrJLSUVoKBmuBA/6xtL4HEs0qM6lGCMcHj l366yPL1qy8CxHrQdooAjHO0t7wRomU0AnP/LiBrNBjmvDifGrzoZ2UVpT9CyJ+dydij in5cedcVaL51sfTvM1Jfhmzd0Sf9lRSW8+Sg00a26ClwgSAqbVMv7oLdzRDsNHf9o5Fo y4Qg== X-Gm-Message-State: ALoCoQk8HhtbEUlT4csWBaeXCF0gjW9Ukxl7bDiurz2NTKPgIAwzFhjQGl20hgSyeGuV1pJJqYde X-Received: by 10.66.229.65 with SMTP id so1mr96197339pac.92.1435282247405; Thu, 25 Jun 2015 18:30:47 -0700 (PDT) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id k5sm31384404pdn.10.2015.06.25.18.30.45 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Jun 2015 18:30:46 -0700 (PDT) Message-ID: <558CAB46.7000201@igel.co.jp> Date: Fri, 26 Jun 2015 10:30:46 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: David Marchand References: <1432014898-3543-2-git-send-email-mukawa@igel.co.jp> <1435202367-8887-1-git-send-email-mukawa@igel.co.jp> <1435202367-8887-4-git-send-email-mukawa@igel.co.jp> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v5 3/5] eal: Fix memory leaks and needless increment of pci_map_addr X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Jun 2015 01:30:48 -0000 On 2015/06/25 18:16, David Marchand wrote: > Hello Tetsuya, > > > On Thu, Jun 25, 2015 at 5:19 AM, Tetsuya Mukawa > wrote: > > From: "Tetsuya.Mukawa" > > > This patch fixes following memory leaks. > - When open() is failed, uio_res and fds won't be freed in > pci_uio_map_resource(). > - When pci_map_resource() is failed but path is allocated correctly, > path and fds won't be freed in pci_uio_map_recource(). > - When pci_uio_unmap() is called, path should be freed. > > Also, fixes below. > - When pci_map_resource() is failed, mapaddr will be MAP_FAILED. > In this case, pci_map_addr should not be incremented in > pci_uio_map_resource(). > - To shrink code, move close(). > - Remove fail variable. > > Signed-off-by: Tetsuya Mukawa > > --- > lib/librte_eal/bsdapp/eal/eal_pci.c | 14 +++++++-- > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 51 > ++++++++++++++++++++----------- > 2 files changed, 44 insertions(+), 21 deletions(-) > > [snip] > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > index 34316b6..2dd83d3 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c > @@ -308,7 +308,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) > if (dev->intr_handle.uio_cfg_fd < 0) { > RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", > cfgname, strerror(errno)); > - return -1; > + goto close_fd; > } > > if (dev->kdrv == RTE_KDRV_IGB_UIO) > > > > You missed a return here : Hi David, Thanks for reviewing It seems I needed to fix it while rebasing. Regards, Tetsuya > > if (dev->kdrv == RTE_KDRV_IGB_UIO) > > dev->intr_handle.type = RTE_INTR_HANDLE_UIO; > > else { > > dev->intr_handle.type = RTE_INTR_HANDLE_UIO_INTX; > > > > /* set bus master that is not done by uio_pci_generic > */ > if > (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) { > RTE_LOG(ERR, EAL, "Cannot set up bus > mastering!\n"); > return -1; > > } > > } > > > > > @@ -328,7 +328,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) > if (uio_res == NULL) { > RTE_LOG(ERR, EAL, > "%s(): cannot store uio mmap details\n", > __func__); > - return -1; > + goto close_fd; > } > > snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname); > @@ -338,7 +338,6 @@ pci_uio_map_resource(struct rte_pci_device *dev) > maps = uio_res->maps; > for (i = 0, map_idx = 0; i != PCI_MAX_RESOURCE; i++) { > int fd; > - int fail = 0; > > /* skip empty BAR */ > phaddr = dev->mem_resource[i].phys_addr; > > > > The rest looks good to me. > > > -- > David Marchand