From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id 30BBE5A9C for ; Mon, 9 Mar 2015 20:30:15 +0100 (CET) Received: from uucp by smtp.tuxdriver.com with local-rmail (Exim 4.63) (envelope-from ) id 1YV3NV-0006gX-NU; Mon, 09 Mar 2015 15:30:13 -0400 Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by localhost.localdomain (8.14.9/8.14.6) with ESMTP id t29JPWap008710; Mon, 9 Mar 2015 15:25:32 -0400 Received: (from linville@localhost) by localhost.localdomain (8.14.9/8.14.9/Submit) id t29JPVlC008709; Mon, 9 Mar 2015 15:25:31 -0400 Date: Mon, 9 Mar 2015 15:25:31 -0400 From: "John W. Linville" To: Ouyang Changchun Message-ID: <20150309192531.GC22009@tuxdriver.com> References: <1425872941-26481-1-git-send-email-changchun.ouyang@intel.com> <1425891491-28779-1-git-send-email-changchun.ouyang@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1425891491-28779-1-git-send-email-changchun.ouyang@intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v5] af_packet: Fix some klocwork errors X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Mar 2015 19:30:15 -0000 On Mon, Mar 09, 2015 at 04:58:11PM +0800, Ouyang Changchun wrote: > Fix possible memory leak issue: free kvlist before return; > Fix possible resource lost issue: close qssockfd before return; > > Signed-off-by: Changchun Ouyang > --- > Change in v5: > - Initialize qsockfd with -1; > > Change in v4: > - Check sockfd in internals->rx_queue against 0. > > Change in v3: > - Also close sockets for all queues. > > Change in v2: > - Make the error exit point a common path. > > lib/librte_pmd_af_packet/rte_eth_af_packet.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/lib/librte_pmd_af_packet/rte_eth_af_packet.c b/lib/librte_pmd_af_packet/rte_eth_af_packet.c > index 80e9bdf..2a27ebc 100644 > --- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c > +++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c > @@ -442,7 +442,8 @@ rte_pmd_init_internals(const char *name, > struct tpacket_req *req; > struct pkt_rx_queue *rx_queue; > struct pkt_tx_queue *tx_queue; > - int rc, qsockfd, tpver, discard; > + int rc, tpver, discard; > + int qsockfd = -1; > unsigned int i, q, rdsize; > int fanout_arg __rte_unused, bypass __rte_unused; > > @@ -691,9 +692,14 @@ error: > rte_free((*internals)->rx_queue[q].rd); > if ((*internals)->tx_queue[q].rd) > rte_free((*internals)->tx_queue[q].rd); > + if (((*internals)->rx_queue[q].sockfd != 0) && > + ((*internals)->rx_queue[q].sockfd != qsockfd)) > + close((*internals)->rx_queue[q].sockfd); > } > rte_free(*internals); > } > + if (qsockfd != -1) > + close(qsockfd); > return -1; > } > The part above seems fine. > @@ -823,16 +829,21 @@ rte_pmd_af_packet_devinit(const char *name, const char *params) > ret = rte_kvargs_process(kvlist, ETH_AF_PACKET_IFACE_ARG, > &open_packet_iface, &sockfd); > if (ret < 0) > - return -1; > + goto error; > } > > ret = rte_eth_from_packet(name, &sockfd, numa_node, kvlist); > close(sockfd); /* no longer needed */ > > if (ret < 0) > - return -1; > + goto error; > > + rte_kvargs_free(kvlist); > return 0; > + > +error: > + rte_kvargs_free(kvlist); > + return -1; > } > > static struct rte_driver pmd_af_packet_drv = { This seems a bit awkward. How about the following? @@ -823,16 +829,15 @@ rte_pmd_af_packet_devinit(const char *name, const char *params) ret = rte_kvargs_process(kvlist, ETH_AF_PACKET_IFACE_ARG, &open_packet_iface, &sockfd); if (ret < 0) - return -1; + goto exit; } ret = rte_eth_from_packet(name, &sockfd, numa_node, kvlist); close(sockfd); /* no longer needed */ - if (ret < 0) - return -1; - - return 0; +exit: + rte_kvargs_free(kvlist); + return ret; } static struct rte_driver pmd_af_packet_drv = { Does this seem OK to you? John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready.