From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi0-f53.google.com (mail-oi0-f53.google.com [209.85.218.53]) by dpdk.org (Postfix) with ESMTP id A33A4C58C for ; Thu, 25 Jun 2015 11:16:09 +0200 (CEST) Received: by oigx81 with SMTP id x81so48153505oig.1 for ; Thu, 25 Jun 2015 02:16:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=x5FE+JgK3FVnPyDd2+zNiSwOuRoB5G5DDjkZzwdF/TA=; b=bE4Oc/Z1v02xt/g+9CPWGMwyDUEkSZ7e8GmahjBLiHf8a/Po7LOCXhAX6VFqhVg/C7 leWjG7yJM8G8MwNW//vDzO16XPOba6PCo/V5bO0SmWrbkkKAZ+hyfdvEcA8dbxCat4Y0 kG8w6Y/kfo1vyXB7CAtugYaMI7GIIwoo6ZWeT4uaicoR/qMhvv4nHR4nri7zPncRcvwc usD4fdaxMnef1Cpu+AHGYlitK3qbgI6E2JSx0TuhprXvpc5C6zi2hvMXkNyuCe0H41fk DEzIWrbIfkZZG+L0wI4/4qgIx5PhZC+D0jn4fDIKrW+hosJZ48zneoA6qgTBQ7dNTMqF 1q6A== X-Gm-Message-State: ALoCoQmvAbiup6yFb8GQV2lJ5T0M9oJGuWuwi9XUNaa2z6D3kZnCGh292kPVX2u5qynAT9qOP3sR MIME-Version: 1.0 X-Received: by 10.202.10.193 with SMTP id 184mr36519017oik.90.1435223769107; Thu, 25 Jun 2015 02:16:09 -0700 (PDT) Received: by 10.76.25.69 with HTTP; Thu, 25 Jun 2015 02:16:09 -0700 (PDT) In-Reply-To: <1435202367-8887-4-git-send-email-mukawa@igel.co.jp> 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> Date: Thu, 25 Jun 2015 11:16:09 +0200 Message-ID: From: David Marchand To: Tetsuya Mukawa Content-Type: text/plain; charset=UTF-8 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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: Thu, 25 Jun 2015 09:16:09 -0000 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 : 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