8:05 a.m.
model was allocated by asprintf but never freed after usage
and assignement.
Signed-off-by: Adam Majer <amajer(a)suse.de>
---
src/Virt_ComputerSystem.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/Virt_ComputerSystem.c b/src/Virt_ComputerSystem.c
index da07f93..b4930ac 100644
--- a/src/Virt_ComputerSystem.c
+++ b/src/Virt_ComputerSystem.c
@@ -417,6 +417,8 @@ static int set_other_id_info(const CMPIBroker *broker,
CMPI_string);
}
+ free(model);
+
CMSetProperty(instance, "OtherIdentifyingInfo",
&id_info, CMPI_stringA);
--
2.13.6
8:05 a.m.
New subject: [libvirt] [cim][PATCH 2/2] Fix memory leak in set_input_props
When caption is specified, cap was allocated and assigned a new pointer
while the old never freed. Store the old pointer in old_cap and free it
upon function exit, if required.
get_input_dev_caption only returns valid cap pointer on success, so
do not try to free it on failure.
Signed-off-by: Adam Majer <amajer(a)suse.de>
---
src/Virt_SettingsDefineCapabilities.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/Virt_SettingsDefineCapabilities.c
b/src/Virt_SettingsDefineCapabilities.c
index 85cb27a..e07de59 100644
--- a/src/Virt_SettingsDefineCapabilities.c
+++ b/src/Virt_SettingsDefineCapabilities.c
@@ -1977,10 +1977,9 @@ static CMPIStatus set_input_props(const CMPIObjectPath *ref,
{
CMPIStatus s = {CMPI_RC_OK, NULL};
CMPIInstance *inst;
- char *cap;
+ char *cap, *old_cap = NULL;
if (get_input_dev_caption(type, bus, &cap) != 1) {
- free(cap);
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_NOT_FOUND,
"Unable to build input caption");
@@ -1988,7 +1987,8 @@ static CMPIStatus set_input_props(const CMPIObjectPath *ref,
}
if (caption != NULL) {
- if (asprintf(&cap, "%s %s", caption, cap) == -1) {
+ old_cap = cap;
+ if (asprintf(&cap, "%s %s", caption, old_cap) == -1) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_NOT_FOUND,
"Unable to build input caption");
@@ -2012,6 +2012,7 @@ static CMPIStatus set_input_props(const CMPIObjectPath *ref,
out:
free(cap);
+ free(old_cap);
return s;
}
--
2.13.6
7:36 a.m.
New subject: [libvirt] [cim][PATCH 2/2] Fix memory leak in set_input_props
On 12/15/2017 09:05 AM, Adam Majer wrote:
When caption is specified, cap was allocated and assigned a new
pointer
while the old never freed. Store the old pointer in old_cap and free it
upon function exit, if required.
get_input_dev_caption only returns valid cap pointer on success, so
do not try to free it on failure.
Signed-off-by: Adam Majer <amajer(a)suse.de>
---
src/Virt_SettingsDefineCapabilities.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
> diff --git a/src/Virt_SettingsDefineCapabilities.c
b/src/Virt_SettingsDefineCapabilities.c
index 85cb27a..e07de59 100644
--- a/src/Virt_SettingsDefineCapabilities.c
+++ b/src/Virt_SettingsDefineCapabilities.c
@@ -1977,10 +1977,9 @@ static CMPIStatus set_input_props(const CMPIObjectPath *ref,
{
CMPIStatus s = {CMPI_RC_OK, NULL};
CMPIInstance *inst;
- char *cap;
+ char *cap, *old_cap = NULL;
Instead of removing, just add "char *tmp;", then....
if (get_input_dev_caption(type, bus, &cap) != 1) {
- free(cap);
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_NOT_FOUND,
"Unable to build input caption");
@@ -1988,7 +1987,8 @@ static CMPIStatus set_input_props(const CMPIObjectPath *ref,
}
if (caption != NULL) {
- if (asprintf(&cap, "%s %s", caption, cap) == -1) {
This should be:
if (&tmp, "%s %s", caption, cap) == -1) {
+ old_cap = cap;
+ if (asprintf(&cap, "%s %s", caption, old_cap) == -1) {
cu_statusf(_BROKER, &s,
CMPI_RC_ERR_NOT_FOUND,
"Unable to build input caption");
and if successful, e.g. after the closing "}"
free(cap)
cap = tmp;
@@ -2012,6 +2012,7 @@ static CMPIStatus set_input_props(const
CMPIObjectPath *ref,
out:
free(cap);
+ free(old_cap);
making this one unnecessary.
John
return s;
}
7:31 a.m.
On 12/15/2017 09:05 AM, Adam Majer wrote:
model was allocated by asprintf but never freed after usage
and assignement.
assignment
Signed-off-by: Adam Majer <amajer(a)suse.de>
---
src/Virt_ComputerSystem.c | 2 ++
1 file changed, 2 insertions(+)
Looks OK; however, there hasn't been a non build related source code
change to libvirt-cim since June 2014. So while this could be pushed,
I'm curious to understand why the "sudden interest" in CIM? The last
release was July 2013!
Perhaps if you'd generated a --cover-letter with your git send-email to
explain that would have helped. See https://libvirt.org/hacking.html for
some general patch submission guidelines when there's more than one
patch in a series, although for libvirt-cim perhaps only the first
section applies as virtually none of the code formatting and naming
would apply...
John
diff --git a/src/Virt_ComputerSystem.c b/src/Virt_ComputerSystem.c
index da07f93..b4930ac 100644
--- a/src/Virt_ComputerSystem.c
+++ b/src/Virt_ComputerSystem.c
@@ -417,6 +417,8 @@ static int set_other_id_info(const CMPIBroker *broker,
CMPI_string);
}
+ free(model);
+
CMSetProperty(instance, "OtherIdentifyingInfo",
&id_info, CMPI_stringA);
5:57 a.m.
On 12/18/2017 02:31 PM, John Ferlan wrote:
Looks OK; however, there hasn't been a non build related source
code
change to libvirt-cim since June 2014. So while this could be pushed,
I'm curious to understand why the "sudden interest" in CIM? The last
release was July 2013!
I'm not sure there is any sudden interest in CIM, but few users have
noticed the slow memory leak. It's better to have this fixed.
Perhaps if you'd generated a --cover-letter with your git
send-email to
explain that would have helped. See https://libvirt.org/hacking.html for
some general patch submission guidelines when there's more than one
patch in a series, although for libvirt-cim perhaps only the first
section applies as virtually none of the code formatting and naming
would apply...
> John
OK, thank you for the tips.
I've updated the patch, as you suggested, to affect less lines of code.
- Adam
2543
days inactive
2568
days old
4 comments
2 participants
participants (2)
-
Adam Majer -
John Ferlan