[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix Resource leak
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-trivial] [Qemu-devel] [PATCH] monitor: Fix Resource leak |
Date: |
Wed, 11 Feb 2015 14:00:03 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
<address@hidden> writes:
> From: Gonglei <address@hidden>
>
> Coverity spot:
> qemu_using_spice allocates memory that is stored into err,
> but not freed before return.
>
> Cc:Markus Armbruster <address@hidden>
> Signed-off-by: Gonglei <address@hidden>
> ---
> 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 <address@hidden>