From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <david.marchand@6wind.com>
Received: from mail-ob0-f175.google.com (mail-ob0-f175.google.com
 [209.85.214.175]) by dpdk.org (Postfix) with ESMTP id 43BCD5A63
 for <dev@dpdk.org>; Tue,  7 Jul 2015 10:04:26 +0200 (CEST)
Received: by obbgp5 with SMTP id gp5so11257548obb.0
 for <dev@dpdk.org>; Tue, 07 Jul 2015 01:04:25 -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=mfiBhNeqGAuDbW/lubdj+aMf+HSLgbL4513DG/wjQS0=;
 b=DRxRtX4/cGxLjnX1XMBhcvzwY7QNtSoJkCfk1L8rGn+3Q3Xeaqa7FQT9gRAiT7cc5R
 zWnXIaMAuuECa93TZxkrbUvb/NHbUZn4VTMS1oQM1HZt7fK9IZ8oqfI5gpyaQW6YrDmw
 6a6DhqM0taK0UbLd4gDOeuSBWtuihI3fqU7w/m28Uu4Ir9TYeIdOR/ggtFZd8ZPKy7v3
 D3omdoJR8cZg5CEbe3ostZxY94hHJoRlX0Y/Z0GQLK8Z0bn2KfAhnX/bBrM48ClptO8Y
 cZv7RSb/reYfAebshZ0XKViYvnf+seDcRQh9U7c5A4hx5TVuPFzaItk7yKXOM+PMYdYO
 3EQg==
X-Gm-Message-State: ALoCoQk/JQu9wDYzlYuNNaBj2Imsx0ZLqtTlWQmxa78ANCjSbhu15/MLpH34V96qboTC3xYIrnkH
MIME-Version: 1.0
X-Received: by 10.202.216.135 with SMTP id p129mr2574210oig.90.1436256265776; 
 Tue, 07 Jul 2015 01:04:25 -0700 (PDT)
Received: by 10.76.84.233 with HTTP; Tue, 7 Jul 2015 01:04:25 -0700 (PDT)
In-Reply-To: <1436163861-3025-4-git-send-email-mukawa@igel.co.jp>
References: <1435652668-3380-12-git-send-email-mukawa@igel.co.jp>
 <1436163861-3025-1-git-send-email-mukawa@igel.co.jp>
 <1436163861-3025-4-git-send-email-mukawa@igel.co.jp>
Date: Tue, 7 Jul 2015 10:04:25 +0200
Message-ID: <CALwxeUvnqbw7-RU7ujs+WVhmwJQee=mRiRQtjcTKK6YSKNx=wA@mail.gmail.com>
From: David Marchand <david.marchand@6wind.com>
To: Tetsuya Mukawa <mukawa@igel.co.jp>
Content-Type: text/plain; charset=UTF-8
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v8 03/12] 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 <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Tue, 07 Jul 2015 08:04:26 -0000

Hello Tetsuya,

Little comment below for this patch.

On Mon, Jul 6, 2015 at 8:24 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:

>
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c
> b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index a63d450..63758c7 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -202,7 +202,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>         uint64_t offset;
>         uint64_t pagesz;
>         struct rte_pci_addr *loc = &dev->addr;
> -       struct uio_resource *uio_res;
> +       struct uio_resource *uio_res = NULL;
>         struct uio_res_list *uio_res_list =
>                         RTE_TAILQ_CAST(rte_uio_tailq.head, uio_res_list);
>         struct uio_map *maps;
> [snip]
> @@ -275,6 +274,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>         TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
>
>         return 0;
> +
> +error:
> +       if (uio_res)
> +               rte_free(uio_res);
>

As long as uio_res is initialized to NULL, no need to check.
rte_free() behaves the same as free().


diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 37dc936..4e50533 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> [snip]
> @@ -400,6 +397,25 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>         TAILQ_INSERT_TAIL(uio_res_list, uio_res, next);
>
>         return 0;
> +
> +error:
> +       for (i = 0; i < map_idx; i++) {
> +               pci_unmap_resource(uio_res->maps[i].addr,
> +                               (size_t)uio_res->maps[i].size);
> +               rte_free(maps[i].path);
> +       }
> +       if (uio_res)
> +               rte_free(uio_res);
>

Idem.


With this,
Acked-by: David Marchand <david.marchand@6wind.com>

-- 
David Marchand