Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in rte_pmd_ring_devuninit()

["Mauricio Vásquez" <mauricio.vasquezbernal@studenti.polito.it>]

On 20 November 2015 at 13:42, Richardson, Bruce <bruce.richardson@intel.com>
wrote:

>
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mcnamara, John
> > Sent: Friday, November 20, 2015 12:32 PM
> > To: Mauricio Vasquez B <mauricio.vasquezbernal@studenti.polito.it>;
> > dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> > rte_pmd_ring_devuninit()
> >
> >
> >
> > > -----Original Message-----
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Mauricio Vasquez
> > > B
> > > Sent: Wednesday, November 18, 2015 10:29 PM
> > > To: dev@dpdk.org
> > > Subject: [dpdk-dev] [PATCH] ring: Fix memory leakage in
> > > rte_pmd_ring_devuninit()
> > >
> > > When freeing the device, it is also necessary to free rx_queues and
> > > tx_queues
> > >
> > > Signed-off-by: Mauricio Vasquez B
> > > <mauricio.vasquezbernal@studenti.polito.it>
> > > ---
> > >  drivers/net/ring/rte_eth_ring.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/drivers/net/ring/rte_eth_ring.c
> > > b/drivers/net/ring/rte_eth_ring.c index 9a31bce..e091e4f 100644
> > > --- a/drivers/net/ring/rte_eth_ring.c
> > > +++ b/drivers/net/ring/rte_eth_ring.c
> > > @@ -582,6 +582,9 @@ rte_pmd_ring_devuninit(const char *name)
> > >             return -ENODEV;
> > >
> > >     eth_dev_stop(eth_dev);
> > > +
> > > +   rte_free(eth_dev->data->rx_queues);
> > > +   rte_free(eth_dev->data->tx_queues);
> > >     rte_free(eth_dev->data->dev_private);
> > >     rte_free(eth_dev->data);
> >
> > Hi,
> >
> > This should test for eth_dev->data before freeing the members. Like:
> >
> >     if (eth_dev->data) {
> >         rte_free(eth_dev->data->rx_queues);
> >         rte_free(eth_dev->data->tx_queues);
> >         rte_free(eth_dev->data->dev_private);
> >     }
> >
> > Thanks,
> >
> > John
>
> That was my thought initially too, but since this is an uninit routine,
> the data field must already have been set up correctly by the init/creation
> function.
>
That was my reasoning too.

> That being said, the check does no harm, so we might as well add it.
>
I agree, I will send a new patch version.

>
> /Bruce
>

Mauricio V,