From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f54.google.com (mail-pa0-f54.google.com [209.85.220.54]) by dpdk.org (Postfix) with ESMTP id 7D79E5A86 for ; Tue, 17 Mar 2015 04:53:43 +0100 (CET) Received: by pacwe9 with SMTP id we9so84687605pac.1 for ; Mon, 16 Mar 2015 20:53:42 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:message-id:date:from:user-agent:mime-version:to :cc:subject:references:in-reply-to:content-type :content-transfer-encoding; bh=S7xhV9oFa86An0Ju9pT/eHM1G7GGrtAF9KVF4QlYxs0=; b=dzloWgkjAV20heZclt0ctdSXuwPLNWHn5MbPDFGTj34qMaF4l0I1+lTExkv4srxNaQ jr7OGqo8AB4+JG+UhKtUOQ5dgJjA8nM1EiJk33u9TbUtacYamqrAfoa1/vZ0SX08wabf HBB69DBiC7c10qw+CZEGhkt4v6pSG3YEA1ca/OvPR04ELc1lsWKQ8BSLNp/+u/JWrAUP tMP+gdDVeRH/uxc/ZlFEJUsWF85yJlvLseU2M8qfZta+lnL35kT6OUuRJniWgZuJJDdN 0t+SqHZ+tgM1ZzQ6ecG7Gd3yKkf86KuV0isLzu50JJvfutT24kzZRWHFv2/IpgmUTGFS lkYg== X-Gm-Message-State: ALoCoQltYl+N6Sd2vaTaI0DmE3v5DIhJgl1QO0vVVp5dw13nhsxDCDgzmzHcdcCVxZuOMV8d/7tV X-Received: by 10.70.46.41 with SMTP id s9mr597388pdm.37.1426564422749; Mon, 16 Mar 2015 20:53:42 -0700 (PDT) Received: from [10.16.129.101] (napt.igel.co.jp. [219.106.231.132]) by mx.google.com with ESMTPSA id qh6sm19731661pab.34.2015.03.16.20.53.41 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 16 Mar 2015 20:53:42 -0700 (PDT) Message-ID: <5507A546.4050307@igel.co.jp> Date: Tue, 17 Mar 2015 12:53:42 +0900 From: Tetsuya Mukawa User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Neil Horman References: <1426477376-13385-1-git-send-email-mukawa@igel.co.jp> <20150316104710.GA16238@hmsreliant.think-freely.org> In-Reply-To: <20150316104710.GA16238@hmsreliant.think-freely.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH] pcap, af_packet, null: Fix memory leaks of 'struct rte_kvargs' 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: Tue, 17 Mar 2015 03:53:44 -0000 On 2015/03/16 19:47, Neil Horman wrote: > On Mon, Mar 16, 2015 at 12:42:56PM +0900, Tetsuya Mukawa wrote: >> 'struct rte_kvargs' is allocated in rte_kvargs_parse(), and should be >> freed with rte_kvargs_free(). >> This patch fixes memory leaks of 'struct rte_kvargs' in below PMDs. >> - pcap PMD >> - af_packet PMD >> - null PMD >> >> Reported-by: Mcnamara, John >> Signed-off-by: Tetsuya Mukawa > Typically, you should limit your goto tags to something more minimalist= that > describes the one thing you want to unwind at the label. As it stands = here, if > somone wants to add some code after this label, the label either become= s > incorrect, or you need to fix up the label name to match. But I suppos= e that > can be corrected later. Hi Neil, I appreciate for your suggestion. I will need to remove af_packet fixing and submit the patch again. So I will follow your suggestion in the next patch. Thanks, Tetsuya > Acked-by: Neil Horman > > >> --- >> lib/librte_pmd_af_packet/rte_eth_af_packet.c | 5 ++++- >> lib/librte_pmd_null/rte_eth_null.c | 13 +++++++++---- >> lib/librte_pmd_pcap/rte_eth_pcap.c | 18 +++++++++++------- >> 3 files changed, 24 insertions(+), 12 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..7c3c6a9 100644 >> --- a/lib/librte_pmd_af_packet/rte_eth_af_packet.c >> +++ b/lib/librte_pmd_af_packet/rte_eth_af_packet.c >> @@ -823,12 +823,15 @@ rte_pmd_af_packet_devinit(const char *name, cons= t char *params) >> ret =3D rte_kvargs_process(kvlist, ETH_AF_PACKET_IFACE_ARG, >> &open_packet_iface, &sockfd); >> if (ret < 0) >> - return -1; >> + goto free_kvlist_then_return; >> } >> =20 >> ret =3D rte_eth_from_packet(name, &sockfd, numa_node, kvlist); >> close(sockfd); /* no longer needed */ >> =20 >> +free_kvlist_then_return: >> + rte_kvargs_free(kvlist); >> + >> if (ret < 0) >> return -1; >> =20 >> diff --git a/lib/librte_pmd_null/rte_eth_null.c b/lib/librte_pmd_null/= rte_eth_null.c >> index f49d686..86307eb 100644 >> --- a/lib/librte_pmd_null/rte_eth_null.c >> +++ b/lib/librte_pmd_null/rte_eth_null.c >> @@ -506,7 +506,7 @@ rte_pmd_null_devinit(const char *name, const char = *params) >> unsigned numa_node; >> unsigned packet_size =3D default_packet_size; >> unsigned packet_copy =3D default_packet_copy; >> - struct rte_kvargs *kvlist; >> + struct rte_kvargs *kvlist =3D NULL; >> int ret; >> =20 >> if (name =3D=3D NULL) >> @@ -527,7 +527,7 @@ rte_pmd_null_devinit(const char *name, const char = *params) >> ETH_NULL_PACKET_SIZE_ARG, >> &get_packet_size_arg, &packet_size); >> if (ret < 0) >> - return -1; >> + goto free_kvlist_then_return; >> } >> =20 >> if (rte_kvargs_count(kvlist, ETH_NULL_PACKET_COPY_ARG) =3D=3D 1) { >> @@ -536,7 +536,7 @@ rte_pmd_null_devinit(const char *name, const char = *params) >> ETH_NULL_PACKET_COPY_ARG, >> &get_packet_copy_arg, &packet_copy); >> if (ret < 0) >> - return -1; >> + goto free_kvlist_then_return; >> } >> } >> =20 >> @@ -544,7 +544,12 @@ rte_pmd_null_devinit(const char *name, const char= *params) >> "packet copy is %s\n", packet_size, >> packet_copy ? "enabled" : "disabled"); >> =20 >> - return eth_dev_null_create(name, numa_node, packet_size, packet_copy= ); >> + ret =3D eth_dev_null_create(name, numa_node, packet_size, packet_cop= y); >> + >> +free_kvlist_then_return: >> + if (kvlist) >> + rte_kvargs_free(kvlist); >> + return ret; >> } >> =20 >> static int >> diff --git a/lib/librte_pmd_pcap/rte_eth_pcap.c b/lib/librte_pmd_pcap/= rte_eth_pcap.c >> index 5e94930..de15fb3 100644 >> --- a/lib/librte_pmd_pcap/rte_eth_pcap.c >> +++ b/lib/librte_pmd_pcap/rte_eth_pcap.c >> @@ -888,12 +888,13 @@ rte_pmd_pcap_devinit(const char *name, const cha= r *params) >> ret =3D rte_kvargs_process(kvlist, ETH_PCAP_IFACE_ARG, >> &open_rx_tx_iface, &pcaps); >> if (ret < 0) >> - return -1; >> + goto free_kvlist_then_return; >> dumpers.pcaps[0] =3D pcaps.pcaps[0]; >> dumpers.names[0] =3D pcaps.names[0]; >> dumpers.types[0] =3D pcaps.types[0]; >> - return rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1, >> + ret =3D rte_eth_from_pcaps(name, &pcaps, 1, &dumpers, 1, >> numa_node, kvlist, 1); >> + goto free_kvlist_then_return; >> } >> =20 >> /* >> @@ -911,7 +912,7 @@ rte_pmd_pcap_devinit(const char *name, const char = *params) >> } >> =20 >> if (ret < 0) >> - return -1; >> + goto free_kvlist_then_return; >> =20 >> /* >> * We check whether we want to open a TX stream to a real NIC or a >> @@ -930,15 +931,18 @@ rte_pmd_pcap_devinit(const char *name, const cha= r *params) >> } >> =20 >> if (ret < 0) >> - return -1; >> + goto free_kvlist_then_return; >> =20 >> if (using_dumpers) >> - return rte_eth_from_pcaps_n_dumpers(name, &pcaps, pcaps.num_of_rx, >> + ret =3D rte_eth_from_pcaps_n_dumpers(name, &pcaps, pcaps.num_of_rx,= >> &dumpers, dumpers.num_of_tx, numa_node, kvlist); >> - >> - return rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_rx, &dumpers, >> + else >> + ret =3D rte_eth_from_pcaps(name, &pcaps, pcaps.num_of_rx, &dumpers,= >> dumpers.num_of_tx, numa_node, kvlist, 0); >> =20 >> +free_kvlist_then_return: >> + rte_kvargs_free(kvlist); >> + return ret; >> } >> =20 >> static int >> --=20 >> 1.9.1 >> >>