> -----Original Message----- > From: Chautru, Nicolas [mailto:nicolas.chau...@intel.com] > Sent: Thursday, October 8, 2020 7:45 AM > To: wangyunjian <wangyunj...@huawei.com>; dev@dpdk.org > Cc: Lilijun (Jerry) <jerry.lili...@huawei.com>; xudingke > <xudin...@huawei.com>; sta...@dpdk.org > Subject: RE: [dpdk-dev] [PATCH] baseband/turbo_sw: fix memory leak in error > path > > Hi wangyunjian, > > > -----Original Message----- > > From: wangyunjian <wangyunj...@huawei.com> > > Sent: Wednesday, October 7, 2020 2:04 AM > > To: dev@dpdk.org > > Cc: Chautru, Nicolas <nicolas.chau...@intel.com>; > > jerry.lili...@huawei.com; xudin...@huawei.com; Yunjian Wang > > <wangyunj...@huawei.com>; sta...@dpdk.org > > Subject: [dpdk-dev] [PATCH] baseband/turbo_sw: fix memory leak in > > error path > > > > From: Yunjian Wang <wangyunj...@huawei.com> > > > > In q_setup() allocated memory for the queue data, we should free it > > when error happens, otherwise it will lead to memory leak. > > > > Fixes: b8cfe2c9aed2 ("bb/turbo_sw: add software turbo driver") > > Cc: sta...@dpdk.org > > > > Signed-off-by: Yunjian Wang <wangyunj...@huawei.com> > > --- > > drivers/baseband/turbo_sw/bbdev_turbo_software.c | 16 > > ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/baseband/turbo_sw/bbdev_turbo_software.c > > b/drivers/baseband/turbo_sw/bbdev_turbo_software.c > > index a36099e91..e55b32927 100644 > > --- a/drivers/baseband/turbo_sw/bbdev_turbo_software.c > > +++ b/drivers/baseband/turbo_sw/bbdev_turbo_software.c > > @@ -302,7 +302,7 @@ q_setup(struct rte_bbdev *dev, uint16_t q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > It may be better to move the freeing into a common function and return the > relevant failure ENUM for each failure reason. > With the proposed changed it would always return EFAULT to application. > > For information did you ever catch that exception from actually running the > code or purely from code review? I struggle to see that error genuinely > happening.
By code review. I will fix return code in the next version. Thanks, Yunjian > > Thanks, > Nic > > > } > > q->enc_out = rte_zmalloc_socket(name, > > ((RTE_BBDEV_TURBO_MAX_TB_SIZE >> 3) + 3) * @@ > > -322,7 +322,7 @@ q_setup(struct rte_bbdev *dev, uint16_t q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->enc_in = rte_zmalloc_socket(name, > > (RTE_BBDEV_LDPC_MAX_CB_SIZE >> 3) * sizeof(*q- > > >enc_in), @@ -340,7 +340,7 @@ q_setup(struct rte_bbdev *dev, uint16_t > > q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->ag = rte_zmalloc_socket(name, > > RTE_BBDEV_TURBO_MAX_CB_SIZE * 10 * sizeof(*q- > > >ag), @@ -358,7 +358,7 @@ q_setup(struct rte_bbdev *dev, uint16_t > > >q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->code_block = rte_zmalloc_socket(name, > > RTE_BBDEV_TURBO_MAX_CB_SIZE * sizeof(*q- > > >code_block), @@ -377,7 +377,7 @@ q_setup(struct rte_bbdev *dev, > > uint16_t q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->deint_input = rte_zmalloc_socket(name, > > DEINT_INPUT_BUF_SIZE * sizeof(*q->deint_input), @@ > > -396,7 > +396,7 > > @@ q_setup(struct rte_bbdev *dev, uint16_t q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->deint_output = rte_zmalloc_socket(NULL, > > DEINT_OUTPUT_BUF_SIZE * sizeof(*q- > > >deint_output), @@ -415,7 +415,7 @@ q_setup(struct rte_bbdev *dev, > > uint16_t q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->adapter_output = rte_zmalloc_socket(NULL, > > ADAPTER_OUTPUT_BUF_SIZE * sizeof(*q- > > >adapter_output), @@ -433,7 +433,7 @@ q_setup(struct rte_bbdev *dev, > > uint16_t q_id, > > rte_bbdev_log(ERR, > > "Creating queue name for device %u queue %u > > failed", > > dev->data->dev_id, q_id); > > - return -ENAMETOOLONG; > > + goto free_q; > > } > > q->processed_pkts = rte_ring_create(name, queue_conf- > > >queue_size, > > queue_conf->socket, RING_F_SP_ENQ | RING_F_SC_DEQ); > > -- > > 2.23.0
