From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>, thomas@monjalon.net
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] eal: fix resource leak
Date: Wed, 4 Oct 2017 20:24:12 +0100 [thread overview]
Message-ID: <27670627-01f0-7fff-0aa8-c9a9d2b89e3d@intel.com> (raw)
In-Reply-To: <20170922144820.16590-1-danielx.t.mrzyglod@intel.com>
On 9/22/2017 3:48 PM, Daniel Mrzyglod wrote:
> Memory allocated in strdup is not free.
>
> Coverity issue: 143257
> Fixes: d8a2bc71dfc2 ("log: remove app path from syslog id")
> Cc: thomas@monjalon.net
>
> Signed-off-by: Daniel Mrzyglod <danielx.t.mrzyglod@intel.com>
> ---
> v2:
> * Fix due to compilation errors
>
> lib/librte_eal/linuxapp/eal/eal.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 48f12f4..a7df566 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -751,7 +751,7 @@ rte_eal_init(int argc, char **argv)
> int i, fctret, ret;
> pthread_t thread_id;
> static rte_atomic32_t run_once = RTE_ATOMIC32_INIT(0);
> - const char *logid;
> + char *logid;
> char cpuset[RTE_CPU_AFFINITY_STR_LEN];
> char thread_name[RTE_MAX_THREAD_NAME_LEN];
>
> @@ -781,6 +781,7 @@ rte_eal_init(int argc, char **argv)
> if (rte_eal_cpu_init() < 0) {
> rte_eal_init_alert("Cannot detect lcores.");
> rte_errno = ENOTSUP;
> + free(logid);
Hi Daniel,
This works but this variable is a nuance and adding free() for this it
into main eal features fail path looks like noise.
Initially, do we need to strdup this variable at all?
What will happen if logid fed into rte_eal_log_init() without strdup?
Since it is const char *, I guess the string is just for read and
content won't be changed so it should be OK I guess.
If above is not right, what about creating a static variable and use it
instead of dynamically allocating the logid, what do you think?
Thanks,
ferruh
> return -1;
> }
>
> @@ -789,6 +790,7 @@ rte_eal_init(int argc, char **argv)
> rte_eal_init_alert("Invalid 'command line' arguments.");
> rte_errno = EINVAL;
> rte_atomic32_clear(&run_once);
> + free(logid);
> return -1;
> }
>
<...>
next prev parent reply other threads:[~2017-10-04 19:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-19 13:34 [dpdk-dev] [PATCH] " Daniel Mrzyglod
2017-09-22 14:48 ` [dpdk-dev] [PATCH v2] " Daniel Mrzyglod
2017-10-02 14:16 ` Jastrzebski, MichalX K
2017-10-04 19:24 ` Ferruh Yigit [this message]
2017-10-05 22:33 ` Thomas Monjalon
2017-10-11 11:53 ` [dpdk-dev] [PATCH v3] " Daniel Mrzyglod
2017-10-11 12:43 ` [dpdk-dev] [dpdk-stable] " Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=27670627-01f0-7fff-0aa8-c9a9d2b89e3d@intel.com \
--to=ferruh.yigit@intel.com \
--cc=danielx.t.mrzyglod@intel.com \
--cc=dev@dpdk.org \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).