tests: check-qom-proplist: fix leak

[Qemu-devel] [PATCH] tests: check-qom-proplist: fix leak

Posted by Marc-André Lureau 8 years, 4 months ago

user_creatable_add_opts() returns a reference (the other reference is
for the root parent/child link).

Leak introduced in commit a1af255f065cc.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/check-qom-proplist.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 8e432e9ab6..a3cd7ab29f 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -428,6 +428,8 @@ static void test_dummy_createcmdl(void)
     g_assert(err == NULL);
     error_free(err);
 
+    object_unref(OBJECT(dobj));
+
     /*
      * cmdline-parsing via qemu_opts_parse() results in a QemuOpts entry
      * corresponding to the Object's ID to be added to the QemuOptsList
-- 
2.13.0.91.g00982b8dd

Re: [Qemu-devel] [PATCH] tests: check-qom-proplist: fix leak

Posted by Eric Blake 8 years, 4 months ago

On 06/09/2017 08:33 AM, Marc-André Lureau wrote:
> user_creatable_add_opts() returns a reference (the other reference is
> for the root parent/child link).
> 
> Leak introduced in commit a1af255f065cc.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/check-qom-proplist.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 8e432e9ab6..a3cd7ab29f 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -428,6 +428,8 @@ static void test_dummy_createcmdl(void)
>      g_assert(err == NULL);
>      error_free(err);
>  
> +    object_unref(OBJECT(dobj));

This should be spelled QDECREF(dobj);

(Hmm - maybe scripts/coccinelle/qobject.cocci should be taught this
cleanup pattern...)

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] tests: check-qom-proplist: fix leak

Posted by Markus Armbruster 8 years, 4 months ago

Eric Blake <eblake@redhat.com> writes:

> On 06/09/2017 08:33 AM, Marc-André Lureau wrote:
>> user_creatable_add_opts() returns a reference (the other reference is
>> for the root parent/child link).
>> 
>> Leak introduced in commit a1af255f065cc.
>> 
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  tests/check-qom-proplist.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
>> index 8e432e9ab6..a3cd7ab29f 100644
>> --- a/tests/check-qom-proplist.c
>> +++ b/tests/check-qom-proplist.c
>> @@ -428,6 +428,8 @@ static void test_dummy_createcmdl(void)
>>      g_assert(err == NULL);
>>      error_free(err);
>>  
>> +    object_unref(OBJECT(dobj));
>
> This should be spelled QDECREF(dobj);

Caution, this is object_unref(), not qobject_decref().

We suck at naming.

> (Hmm - maybe scripts/coccinelle/qobject.cocci should be taught this
> cleanup pattern...)

Re: [Qemu-devel] [PATCH] tests: check-qom-proplist: fix leak

Posted by Eric Blake 8 years, 4 months ago

On 06/09/2017 10:34 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
> 
>> On 06/09/2017 08:33 AM, Marc-André Lureau wrote:
>>> user_creatable_add_opts() returns a reference (the other reference is
>>> for the root parent/child link).
>>>
>>> Leak introduced in commit a1af255f065cc.
>>>
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>  tests/check-qom-proplist.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
>>> index 8e432e9ab6..a3cd7ab29f 100644
>>> --- a/tests/check-qom-proplist.c
>>> +++ b/tests/check-qom-proplist.c
>>> @@ -428,6 +428,8 @@ static void test_dummy_createcmdl(void)
>>>      g_assert(err == NULL);
>>>      error_free(err);
>>>  
>>> +    object_unref(OBJECT(dobj));
>>
>> This should be spelled QDECREF(dobj);
> 
> Caution, this is object_unref(), not qobject_decref().

Indeed - now that I re-read it closely, I see that you are right.  Oh
well - I didn't waste much time improving the qobject coccinelle script,
even though this patch proposal is unrelated to that one. :)

> 
> We suck at naming.

Not the first time.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] tests: check-qom-proplist: fix leak

Posted by Markus Armbruster 8 years, 3 months ago

Copying qemu-trivial.

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> user_creatable_add_opts() returns a reference (the other reference is
> for the root parent/child link).
>
> Leak introduced in commit a1af255f065cc.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  tests/check-qom-proplist.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
> index 8e432e9ab6..a3cd7ab29f 100644
> --- a/tests/check-qom-proplist.c
> +++ b/tests/check-qom-proplist.c
> @@ -428,6 +428,8 @@ static void test_dummy_createcmdl(void)
>      g_assert(err == NULL);
>      error_free(err);
>  
> +    object_unref(OBJECT(dobj));
> +
>      /*
>       * cmdline-parsing via qemu_opts_parse() results in a QemuOpts entry
>       * corresponding to the Object's ID to be added to the QemuOptsList

Reviewed-by: Markus Armbruster <armbru@redhat.com>