[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [Qemu-devel] [PATCH 12/15] gluster: Plug memory leaks i
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-block] [Qemu-devel] [PATCH 12/15] gluster: Plug memory leaks in qemu_gluster_parse_json() |
Date: |
Fri, 03 Mar 2017 08:38:45 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Niels de Vos <address@hidden> writes:
> On Thu, Mar 02, 2017 at 10:44:03PM +0100, Markus Armbruster wrote:
>> To reproduce, run
>>
>> $ valgrind qemu-system-x86_64 --nodefaults -S --drive
>> driver=gluster,volume=testvol,path=/a/b/c,server.0.type=xxx
>>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>> block/gluster.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/block/gluster.c b/block/gluster.c
>> index 6fbcf9e..35a7abb 100644
>> --- a/block/gluster.c
>> +++ b/block/gluster.c
>> @@ -480,7 +480,7 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>> QDict *options, Error **errp)
>> {
>> QemuOpts *opts;
>> - GlusterServer *gsconf;
>> + GlusterServer *gsconf = NULL;
>> GlusterServerList *curr = NULL;
>> QDict *backing_options = NULL;
>> Error *local_err = NULL;
>> @@ -529,17 +529,16 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>> }
>>
>> ptr = qemu_opt_get(opts, GLUSTER_OPT_TYPE);
>> + if (!ptr) {
>> + error_setg(&local_err, QERR_MISSING_PARAMETER,
>> GLUSTER_OPT_TYPE);
>> + error_append_hint(&local_err, GERR_INDEX_HINT, i);
>> + goto out;
>> +
>> + }
>> gsconf = g_new0(GlusterServer, 1);
>> gsconf->type = qapi_enum_parse(GlusterTransport_lookup, ptr,
>> - GLUSTER_TRANSPORT__MAX,
>> - GLUSTER_TRANSPORT__MAX,
>> + GLUSTER_TRANSPORT__MAX, 0,
>
> What is the reason to set the default to 0 and not the more readable
> GLUSTER_TRANSPORT_UNIX?
I chose 0 because the actual value must not matter: we don't want a
default here.
qapi_enum_parse() returns this argument when ptr isn't found. If ptr is
non-null, it additionally sets an error. Since ptr can't be null here,
the argument is only returned together with an error. But then we won't
use *gsconf.
Do you think -1 instead of 0 would be clearer?
>> &local_err);
>> - if (!ptr) {
>> - error_setg(&local_err, QERR_MISSING_PARAMETER,
>> GLUSTER_OPT_TYPE);
>> - error_append_hint(&local_err, GERR_INDEX_HINT, i);
>> - goto out;
>> -
>> - }
>> if (local_err) {
>> error_append_hint(&local_err,
>> "Parameter '%s' may be 'tcp' or 'unix'\n",
>> @@ -626,8 +625,10 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>> curr->next->value = gsconf;
>> curr = curr->next;
>> }
>> + gsconf = NULL;
>>
>> - qdict_del(backing_options, str);
>> + QDECREF(backing_options);
>> + backing_options = NULL;
>> g_free(str);
>> str = NULL;
>> }
>> @@ -636,11 +637,10 @@ static int
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>>
>> out:
>> error_propagate(errp, local_err);
>> + qapi_free_GlusterServer(gsconf);
>> qemu_opts_del(opts);
>> - if (str) {
>> - qdict_del(backing_options, str);
>> - g_free(str);
>> - }
>> + g_free(str);
>> + QDECREF(backing_options);
>> errno = EINVAL;
>> return -errno;
>> }
>> --
>> 2.7.4
>>
>>
[Qemu-block] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create(), Markus Armbruster, 2017/03/02
Re: [Qemu-block] [Qemu-devel] [PATCH 06/15] sheepdog: Don't truncate long VDI name in _open(), _create(), Philippe Mathieu-Daudé, 2017/03/02
[Qemu-block] [PATCH 02/15] sheepdog: Fix error handling in sd_snapshot_delete(), Markus Armbruster, 2017/03/02