* [Qemu-trivial] [PATCH] monitor: Fix Resource leak
@ 2015-02-11 6:26 ` arei.gonglei
0 siblings, 0 replies; 6+ messages in thread
From: arei.gonglei @ 2015-02-11 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Gonglei, lcapitulino
From: Gonglei <arei.gonglei@huawei.com>
Coverity spot:
qemu_using_spice allocates memory that is stored into err,
but not freed before return.
Cc:Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
monitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/monitor.c b/monitor.c
index c3cc060..137d23f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
const char *subject = qdict_get_try_str(qdict, "cert-subject");
int port = qdict_get_try_int(qdict, "port", -1);
int tls_port = qdict_get_try_int(qdict, "tls-port", -1);
- Error *err;
+ Error *err = NULL;
int ret;
if (strcmp(protocol, "spice") == 0) {
if (!qemu_using_spice(&err)) {
qerror_report_err(err);
+ error_free(err);
return -1;
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH] monitor: Fix Resource leak
@ 2015-02-11 6:26 ` arei.gonglei
0 siblings, 0 replies; 6+ messages in thread
From: arei.gonglei @ 2015-02-11 6:26 UTC (permalink / raw)
To: qemu-devel; +Cc: qemu-trivial, Gonglei, lcapitulino
From: Gonglei <arei.gonglei@huawei.com>
Coverity spot:
qemu_using_spice allocates memory that is stored into err,
but not freed before return.
Cc:Markus Armbruster <armbru@redhat.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
monitor.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/monitor.c b/monitor.c
index c3cc060..137d23f 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
const char *subject = qdict_get_try_str(qdict, "cert-subject");
int port = qdict_get_try_int(qdict, "port", -1);
int tls_port = qdict_get_try_int(qdict, "tls-port", -1);
- Error *err;
+ Error *err = NULL;
int ret;
if (strcmp(protocol, "spice") == 0) {
if (!qemu_using_spice(&err)) {
qerror_report_err(err);
+ error_free(err);
return -1;
}
--
1.7.12.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix Resource leak
2015-02-11 6:26 ` [Qemu-devel] " arei.gonglei
@ 2015-02-11 13:00 ` Markus Armbruster
-1 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-02-11 13:00 UTC (permalink / raw)
To: arei.gonglei; +Cc: qemu-trivial, qemu-devel, lcapitulino
<arei.gonglei@huawei.com> writes:
> From: Gonglei <arei.gonglei@huawei.com>
>
> Coverity spot:
> qemu_using_spice allocates memory that is stored into err,
> but not freed before return.
>
> Cc:Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> monitor.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/monitor.c b/monitor.c
> index c3cc060..137d23f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
> const char *subject = qdict_get_try_str(qdict, "cert-subject");
> int port = qdict_get_try_int(qdict, "port", -1);
> int tls_port = qdict_get_try_int(qdict, "tls-port", -1);
> - Error *err;
> + Error *err = NULL;
> int ret;
>
> if (strcmp(protocol, "spice") == 0) {
> if (!qemu_using_spice(&err)) {
> qerror_report_err(err);
> + error_free(err);
> return -1;
> }
Your commit message is incomplete. The resource leak is real, but it's
the less serious bug here. The serious one is missing initialization of
err.
If using_spice, qemu_using_spice() returns true without touching err.
All fine.
Else, if err is null by chance, qemu_using_spice()'s error_set() creates
an error object and stores it in err. We leak it.
If err is not null, error_set() fails its assertion.
Broken in commit b25d81b by yours truly. Thanks for cleaning up my
mess!
Let's fix up the commit message:
monitor: Fix missing err = NULL in client_migrate_info()
When SPICE isn't used, we either fail an assertion in error_set(),
or leak an error object. Broken in commit b25d81b.
With such a fixup:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] monitor: Fix Resource leak
@ 2015-02-11 13:00 ` Markus Armbruster
0 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2015-02-11 13:00 UTC (permalink / raw)
To: arei.gonglei; +Cc: qemu-trivial, qemu-devel, lcapitulino
<arei.gonglei@huawei.com> writes:
> From: Gonglei <arei.gonglei@huawei.com>
>
> Coverity spot:
> qemu_using_spice allocates memory that is stored into err,
> but not freed before return.
>
> Cc:Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
> monitor.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/monitor.c b/monitor.c
> index c3cc060..137d23f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
> const char *subject = qdict_get_try_str(qdict, "cert-subject");
> int port = qdict_get_try_int(qdict, "port", -1);
> int tls_port = qdict_get_try_int(qdict, "tls-port", -1);
> - Error *err;
> + Error *err = NULL;
> int ret;
>
> if (strcmp(protocol, "spice") == 0) {
> if (!qemu_using_spice(&err)) {
> qerror_report_err(err);
> + error_free(err);
> return -1;
> }
Your commit message is incomplete. The resource leak is real, but it's
the less serious bug here. The serious one is missing initialization of
err.
If using_spice, qemu_using_spice() returns true without touching err.
All fine.
Else, if err is null by chance, qemu_using_spice()'s error_set() creates
an error object and stores it in err. We leak it.
If err is not null, error_set() fails its assertion.
Broken in commit b25d81b by yours truly. Thanks for cleaning up my
mess!
Let's fix up the commit message:
monitor: Fix missing err = NULL in client_migrate_info()
When SPICE isn't used, we either fail an assertion in error_set(),
or leak an error object. Broken in commit b25d81b.
With such a fixup:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix Resource leak
2015-02-11 13:00 ` Markus Armbruster
@ 2015-02-12 1:04 ` Gonglei
-1 siblings, 0 replies; 6+ messages in thread
From: Gonglei @ 2015-02-12 1:04 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-trivial, qemu-devel, lcapitulino
On 2015/2/11 21:00, Markus Armbruster wrote:
> <arei.gonglei@huawei.com> writes:
>
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Coverity spot:
>> qemu_using_spice allocates memory that is stored into err,
>> but not freed before return.
>>
>> Cc:Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>> monitor.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c3cc060..137d23f 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
>> const char *subject = qdict_get_try_str(qdict, "cert-subject");
>> int port = qdict_get_try_int(qdict, "port", -1);
>> int tls_port = qdict_get_try_int(qdict, "tls-port", -1);
>> - Error *err;
>> + Error *err = NULL;
>> int ret;
>>
>> if (strcmp(protocol, "spice") == 0) {
>> if (!qemu_using_spice(&err)) {
>> qerror_report_err(err);
>> + error_free(err);
>> return -1;
>> }
>
> Your commit message is incomplete. The resource leak is real, but it's
> the less serious bug here. The serious one is missing initialization of
> err.
>
> If using_spice, qemu_using_spice() returns true without touching err.
> All fine.
>
> Else, if err is null by chance, qemu_using_spice()'s error_set() creates
> an error object and stores it in err. We leak it.
>
> If err is not null, error_set() fails its assertion.
>
Thanks for your explanation, I haven't a global inspection TBH.
> Broken in commit b25d81b by yours truly. Thanks for cleaning up my
> mess!
>
> Let's fix up the commit message:
>
> monitor: Fix missing err = NULL in client_migrate_info()
>
> When SPICE isn't used, we either fail an assertion in error_set(),
> or leak an error object. Broken in commit b25d81b.
>
OK, will fix.
> With such a fixup:
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
Thanks.
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Qemu-devel] [PATCH] monitor: Fix Resource leak
@ 2015-02-12 1:04 ` Gonglei
0 siblings, 0 replies; 6+ messages in thread
From: Gonglei @ 2015-02-12 1:04 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-trivial, qemu-devel, lcapitulino
On 2015/2/11 21:00, Markus Armbruster wrote:
> <arei.gonglei@huawei.com> writes:
>
>> From: Gonglei <arei.gonglei@huawei.com>
>>
>> Coverity spot:
>> qemu_using_spice allocates memory that is stored into err,
>> but not freed before return.
>>
>> Cc:Markus Armbruster <armbru@redhat.com>
>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>> ---
>> monitor.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/monitor.c b/monitor.c
>> index c3cc060..137d23f 100644
>> --- a/monitor.c
>> +++ b/monitor.c
>> @@ -1095,12 +1095,13 @@ static int client_migrate_info(Monitor *mon, const QDict *qdict,
>> const char *subject = qdict_get_try_str(qdict, "cert-subject");
>> int port = qdict_get_try_int(qdict, "port", -1);
>> int tls_port = qdict_get_try_int(qdict, "tls-port", -1);
>> - Error *err;
>> + Error *err = NULL;
>> int ret;
>>
>> if (strcmp(protocol, "spice") == 0) {
>> if (!qemu_using_spice(&err)) {
>> qerror_report_err(err);
>> + error_free(err);
>> return -1;
>> }
>
> Your commit message is incomplete. The resource leak is real, but it's
> the less serious bug here. The serious one is missing initialization of
> err.
>
> If using_spice, qemu_using_spice() returns true without touching err.
> All fine.
>
> Else, if err is null by chance, qemu_using_spice()'s error_set() creates
> an error object and stores it in err. We leak it.
>
> If err is not null, error_set() fails its assertion.
>
Thanks for your explanation, I haven't a global inspection TBH.
> Broken in commit b25d81b by yours truly. Thanks for cleaning up my
> mess!
>
> Let's fix up the commit message:
>
> monitor: Fix missing err = NULL in client_migrate_info()
>
> When SPICE isn't used, we either fail an assertion in error_set(),
> or leak an error object. Broken in commit b25d81b.
>
OK, will fix.
> With such a fixup:
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
Thanks.
Regards,
-Gonglei
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-02-12 1:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-11 6:26 [Qemu-trivial] [PATCH] monitor: Fix Resource leak arei.gonglei
2015-02-11 6:26 ` [Qemu-devel] " arei.gonglei
2015-02-11 13:00 ` [Qemu-trivial] " Markus Armbruster
2015-02-11 13:00 ` Markus Armbruster
2015-02-12 1:04 ` [Qemu-trivial] " Gonglei
2015-02-12 1:04 ` Gonglei
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.