Hi Andy, > -----Original Message----- > From: Pei, Andy > Sent: Wednesday, December 19, 2018 3:35 > To: dev@dpdk.org > Cc: Xu, Rosen <rosen...@intel.com>; Zhang, Tianfei > <tianfei.zh...@intel.com>; Pei, Andy <andy....@intel.com> > Subject: [PATCH v2]/driver/raw/ifpga_rawdev: fix a memory leak bug in ifpga > > When ifpga_rawdev_create() allocate memory for a new rawdev, the original > code allocate redundant memory for adapter, which is a member of the > rawdev. What is actually necessary is the adapter to be initialized, not > memory allocated. > > fixes:ef1e8ede3da5 > cc:rosen...@intel.com > cc:tianfei.zh...@intel.com > > Signed-off-by: AndyPei <andy....@intel.com> > --- > drivers/raw/ifpga_rawdev/base/opae_hw_api.c | 32 > ++++++++++++++++++++++++----- > drivers/raw/ifpga_rawdev/base/opae_hw_api.h | 4 +++- > drivers/raw/ifpga_rawdev/ifpga_rawdev.c | 7 +++---- > 3 files changed, 33 insertions(+), 10 deletions(-) > > diff --git a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > index a533dfe..50f6438 100644 > --- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > +++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.c > @@ -303,12 +303,35 @@ static struct opae_adapter_ops *match_ops(struct > opae_adapter *adapter) } > > /** > - * opae_adapter_data_alloc - alloc opae_adapter_data data structure > + * opae_adapter_init - init opae_adapter data structure > + * @adpdate: pointer of opae_adater data structure > + * @name: adapter name. > + * @data: private data of this adapter. > + * > + * Return: 0 on success. > + */ > +int opae_adapter_init(struct opae_adapter *adapter, > + const char *name, void *data) > +{ > + if (!adapter) > + return -ENOMEM; > + > + TAILQ_INIT(&adapter->acc_list); > + adapter->data = data; > + adapter->name = name; > + adapter->ops = match_ops(adapter); > + > + return 0; > +} > + > +/** > + * opae_adapter_alloc - alloc opae_adapter data structure > * @name: adapter name. > * @data: private data of this adapter. > * > * Return: opae_adapter on success, otherwise NULL. > */ > + /**This function will no longer be called. > struct opae_adapter *opae_adapter_alloc(const char *name, void *data) { > struct opae_adapter *adapter = opae_zmalloc(sizeof(*adapter)); > @@ -316,13 +339,12 @@ struct opae_adapter *opae_adapter_alloc(const > char *name, void *data) > if (!adapter) > return NULL; > > - TAILQ_INIT(&adapter->acc_list); > - adapter->data = data; > - adapter->name = name; > - adapter->ops = match_ops(adapter); > + if (opae_adapter_init(adapter, name, data)) > + return NULL; > > return adapter; > } To avoid redundancy, does this function useful? > +**/ > > /** > * opae_adapter_enumerate - enumerate this adapter diff --git > a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > index 4bbc9df..ac1941a 100644 > --- a/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > +++ b/drivers/raw/ifpga_rawdev/base/opae_hw_api.h > @@ -225,7 +225,9 @@ struct opae_adapter { void > *opae_adapter_data_alloc(enum opae_adapter_type type); #define > opae_adapter_data_free(data) opae_free(data) > > -struct opae_adapter *opae_adapter_alloc(const char *name, void *data); > +int opae_adapter_init(struct opae_adapter *adapter, > + const char *name, void *data); > +//struct opae_adapter *opae_adapter_alloc(const char *name, void > +*data); > #define opae_adapter_free(adapter) opae_free(adapter) > > int opae_adapter_enumerate(struct opae_adapter *adapter); diff --git > a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > index 32e318f..d433091 100644 > --- a/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > +++ b/drivers/raw/ifpga_rawdev/ifpga_rawdev.c > @@ -409,9 +409,10 @@ > data->device_id = pci_dev->id.device_id; > data->vendor_id = pci_dev->id.vendor_id; > > + adapter = rawdev->dev_private; > /* create a opae_adapter based on above device data */ > - adapter = opae_adapter_alloc(pci_dev->device.name, data); > - if (!adapter) { > + ret = opae_adapter_init(adapter, pci_dev->device.name, data); > + if (ret) { > ret = -ENOMEM; > goto free_adapter_data;
Return error from opae_adapter_init() will cause free data? Do you need to free data? > } > @@ -420,8 +421,6 @@ > rawdev->device = &pci_dev->device; > rawdev->driver_name = pci_dev->device.driver->name; > > - rawdev->dev_private = adapter; > - > /* must enumerate the adapter before use it */ > ret = opae_adapter_enumerate(adapter); > if (ret) > -- > 1.8.3.1