From mboxrd@z Thu Jan  1 00:00:00 1970
From: Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>
Subject: Re: [PATCH 3/6] eal: Fix memory leaks and needless
 incrementation of pci uio implementation
Date: Fri, 20 Mar 2015 10:53:36 +0900
Message-ID: <550B7DA0.8000109@igel.co.jp>
References: <1426155474-1596-4-git-send-email-mukawa@igel.co.jp>	<1426584645-28828-1-git-send-email-mukawa@igel.co.jp>	<1426584645-28828-4-git-send-email-mukawa@igel.co.jp>
 <CALwxeUsOBQP9joqtKZ4uaZGFTphO1QT9jvJqbqq_Psu-zOAz4g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Cc: "dev-VfR2kkLFssw@public.gmane.org" <dev-VfR2kkLFssw@public.gmane.org>
To: David Marchand <david.marchand-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>
Return-path: <dev-bounces-VfR2kkLFssw@public.gmane.org>
In-Reply-To: <CALwxeUsOBQP9joqtKZ4uaZGFTphO1QT9jvJqbqq_Psu-zOAz4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev-VfR2kkLFssw@public.gmane.org>
List-Help: <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request-VfR2kkLFssw@public.gmane.org?subject=subscribe>
Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org
Sender: "dev" <dev-bounces-VfR2kkLFssw@public.gmane.org>

On 2015/03/19 0:39, David Marchand wrote:
> On Tue, Mar 17, 2015 at 10:30 AM, Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org
> <mailto:mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>> wrote:
>
>     When pci_map_resource() is failed but path is allocated correctly,
>     path won't be freed. Also, when open() is failed, uio_res won't be
>     freed.
>     This patch fixes these memory leaks.
>     When pci_map_resource() is failed, mapaddr will be MAP_FAILED.
>     In this case, pci_map_addr should not be incremented.
>
>     Also, the patch fixes belows.
>      - To shrink code, move close().
>      - Remove fail variable.
>
>     Signed-off-by: Tetsuya Mukawa <mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org
>     <mailto:mukawa-AlSX/UN32fvPDbFq/vQRIQ@public.gmane.org>>
>     ---
>      lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 28
>     ++++++++++++++--------------
>      1 file changed, 14 insertions(+), 14 deletions(-)
>
>     diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>     b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>     index b971ec9..5044884 100644
>     --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>     +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
>     @@ -333,7 +333,6 @@ pci_uio_map_resource(struct rte_pci_device *dev=
)
>             maps =3D uio_res->maps;
>             for (i =3D 0, map_idx =3D 0; i !=3D PCI_MAX_RESOURCE; i++) =
{
>                     int fd;
>     -               int fail =3D 0;
>
>                     /* skip empty BAR */
>                     phaddr =3D dev->mem_resource[i].phys_addr;
>     @@ -347,6 +346,11 @@ pci_uio_map_resource(struct rte_pci_device *de=
v)
>                                     loc->domain, loc->bus, loc->devid,
>     loc->function,
>                                     i);
>
>     +               /* allocate memory to keep path */
>     +               maps[map_idx].path =3D rte_malloc(NULL,
>     strlen(devname) + 1, 0);
>     +               if (maps[map_idx].path =3D=3D NULL)
>     +                       goto fail0;
>     +
>
>
>  [snip]
>
> Neither fail0 nor fail1 labels seem to free previous allocations.
> Did I miss something ?

Hi David,


Ah, you are right. I will free all 'maps[i].path' allocated in this
function.

Thanks,
Tetsuya


>
> [snip]
>
>     +
>     +fail1:
>     +       rte_free(maps[map_idx].path);
>     +fail0:
>     +       rte_free(uio_res);
>     +       return -1;
>      }
>
>
> =20
>
> --=20
> David Marchand