From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ob0-f173.google.com (mail-ob0-f173.google.com [209.85.214.173]) by dpdk.org (Postfix) with ESMTP id 5DAA0C57E for ; Thu, 25 Jun 2015 11:18:19 +0200 (CEST) Received: by obctg8 with SMTP id tg8so43241440obc.3 for ; Thu, 25 Jun 2015 02:18:18 -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=ONw4uzxhfe2oY4ERed5IJhJZ2t3pIHXHH/qqnadhVsE=; b=gP8sql7stK4EGvWd9FuoUodhmRh9+mKUv/R/L7zinyuqliYKb/dfB6jBocMjOrXbd8 hMn7yEPLDrEX6wMQ9136TOR8yk8qhZFOXYOxKmY9lV3dZltONT1KMIxcXoh391HW/sqw 5wl9kAeQCF+VDxpx6qJ74EGd/hN/HEbfawmivb6kfbMMDD5wJi+cAuL4EGr0UZvu+WXd ybFrh+XFlavRN/NYrU4yZK7bNIpq/MFvyzVu7yKSwJJH35Jvg9aUyaE4d/4fwUM3kcGZ iKvoRxGN7I7MGb0MMXhnM3DXXkPxWA8JnMLAzXt0gsmdp4ZuexyY3B/goIikuJDxhl8g xAPw== X-Gm-Message-State: ALoCoQns++AOgniHjafJi3sHFHD9MVJQ6/u4meRFAQtGCyRsnm3ZEyoZWpMiCkzBlpnRTLhMQtzk MIME-Version: 1.0 X-Received: by 10.202.202.200 with SMTP id a191mr7993076oig.118.1435223898886; Thu, 25 Jun 2015 02:18:18 -0700 (PDT) Received: by 10.76.25.69 with HTTP; Thu, 25 Jun 2015 02:18:18 -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:18:18 +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:18:19 -0000 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(-) > > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c > b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 8e24fd1..b071f07 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -235,7 +235,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) > if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == > 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); > @@ -262,8 +262,7 @@ pci_uio_map_resource(struct rte_pci_device *dev) > (mapaddr = pci_map_resource(NULL, devname, > (off_t)offset, > (size_t)maps[j].size) > ) == NULL) { > - rte_free(uio_res); > - return -1; > + goto free_uio_res; > } > > maps[j].addr = mapaddr; > @@ -274,6 +273,15 @@ pci_uio_map_resource(struct rte_pci_device *dev) > TAILQ_INSERT_TAIL(uio_res_list, uio_res, next); > > return 0; > + > +free_uio_res: > + rte_free(uio_res); > +close_fd: > + close(dev->intr_handle.fd); > + dev->intr_handle.fd = -1; > + dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN; > + > + return -1; > } > > Thinking about it, when something fails, don't you need to unmap pci resources in uio_res->maps before freeing ? -- David Marchand