Re: [PATCH v2]/driver/raw/ifpga_rawdev: fix a memory leak bug in ifpga
From: Xu, Rosen <hidden>
Date: 2018-12-19 07:16:32
Hi Andy,
quoted hunk ↗ jump to hunk
-----Original Message----- From: Pei, Andy Sent: Wednesday, December 19, 2018 3:35 To: dev@dpdk.org Cc: Xu, Rosen <redacted>; Zhang, Tianfei [off-list ref]; Pei, Andy [off-list ref] 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.xu@intel.com cc:tianfei.zhang@intel.com Signed-off-by: AndyPei <redacted> --- 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.cb/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(structopae_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(constchar *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?
quoted hunk ↗ jump to hunk
+**/ /** * 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?
quoted hunk ↗ jump to hunk
}@@ -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