[dpdk-dev] [PATCH v5 3/5] eal: Fix memory leaks and needless increment of pci_map_addr

Tetsuya Mukawa mukawa at igel.co.jp
Fri Jun 26 03:30:46 CEST 2015


On 2015/06/25 18:16, David Marchand wrote:
> Hello Tetsuya, 
>
>
> On Thu, Jun 25, 2015 at 5:19 AM, Tetsuya Mukawa <mukawa at igel.co.jp
> <mailto:mukawa at igel.co.jp>> wrote:
>
>     From: "Tetsuya.Mukawa" <mukawa at igel.co.jp <mailto:mukawa at igel.co.jp>>
>
>     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 <mukawa at igel.co.jp
>     <mailto:mukawa at igel.co.jp>>
>     ---
>      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 



More information about the dev mailing list