* [PATCH] vhost-user-test: fix a memory leak
@ 2019-12-11 0:55 pannengyuan
2019-12-11 7:48 ` Laurent Vivier
2019-12-11 7:57 ` Thomas Huth
0 siblings, 2 replies; 7+ messages in thread
From: pannengyuan @ 2019-12-11 0:55 UTC (permalink / raw)
To: thuth, lvivier, pbonzini; +Cc: Pan Nengyuan, zhang.zhanghailiang, qemu-devel
From: Pan Nengyuan <pannengyuan@huawei.com>
Spotted by ASAN.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
---
tests/vhost-user-test.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 91ea373..54be931 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
guint64 size;
if (!wait_for_fds(s)) {
+ g_free(uri);
+ test_server_free(dest);
return;
}
--
2.7.2.windows.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] vhost-user-test: fix a memory leak
2019-12-11 0:55 [PATCH] vhost-user-test: fix a memory leak pannengyuan
@ 2019-12-11 7:48 ` Laurent Vivier
2019-12-12 1:11 ` Pan Nengyuan
2019-12-11 7:57 ` Thomas Huth
1 sibling, 1 reply; 7+ messages in thread
From: Laurent Vivier @ 2019-12-11 7:48 UTC (permalink / raw)
To: pannengyuan, thuth, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel
On 11/12/2019 01:55, pannengyuan@huawei.com wrote:
> From: Pan Nengyuan <pannengyuan@huawei.com>
>
> Spotted by ASAN.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
> ---
> tests/vhost-user-test.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 91ea373..54be931 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
> guint64 size;
>
> if (!wait_for_fds(s)) {
> + g_free(uri);
> + test_server_free(dest);
> return;
> }
>
Don't we need also a g_string_free(dest_cmdline)?
I think it is also missing at the end of the function.
Thanks,
Laurent
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] vhost-user-test: fix a memory leak
2019-12-11 7:48 ` Laurent Vivier
@ 2019-12-12 1:11 ` Pan Nengyuan
0 siblings, 0 replies; 7+ messages in thread
From: Pan Nengyuan @ 2019-12-12 1:11 UTC (permalink / raw)
To: Laurent Vivier, thuth, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel
On 2019/12/11 15:48, Laurent Vivier wrote:
> On 11/12/2019 01:55, pannengyuan@huawei.com wrote:
>> From: Pan Nengyuan <pannengyuan@huawei.com>
>>
>> Spotted by ASAN.
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
>> ---
>> tests/vhost-user-test.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
>> index 91ea373..54be931 100644
>> --- a/tests/vhost-user-test.c
>> +++ b/tests/vhost-user-test.c
>> @@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
>> guint64 size;
>>
>> if (!wait_for_fds(s)) {
>> + g_free(uri);
>> + test_server_free(dest);
>> return;
>> }
>>
>
> Don't we need also a g_string_free(dest_cmdline)?
>
> I think it is also missing at the end of the function.
>
Yes, you are right. But I'm surprised that it hasn't been detected by asan.
I will continue to try it and send a new version.
> Thanks,
> Laurent
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-test: fix a memory leak
2019-12-11 0:55 [PATCH] vhost-user-test: fix a memory leak pannengyuan
2019-12-11 7:48 ` Laurent Vivier
@ 2019-12-11 7:57 ` Thomas Huth
2019-12-11 8:18 ` Marc-André Lureau
2019-12-12 1:41 ` Pan Nengyuan
1 sibling, 2 replies; 7+ messages in thread
From: Thomas Huth @ 2019-12-11 7:57 UTC (permalink / raw)
To: pannengyuan, lvivier, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel
Hi!
On 11/12/2019 01.55, pannengyuan@huawei.com wrote:
[...]
> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> index 91ea373..54be931 100644
> --- a/tests/vhost-user-test.c
> +++ b/tests/vhost-user-test.c
> @@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
> guint64 size;
>
> if (!wait_for_fds(s)) {
> + g_free(uri);
> + test_server_free(dest);
> return;
> }
Well spotted. But I'd prefer to rather move the allocation of these
resources after the if-statement instead of doing the allocation at the
declaration of the variables already. Or maybe use a "goto out" and jump
to the end of the function instead? ... whatever you prefer, but
duplicating the "free" functions sounds like a cumbersome solution to me.
Thanks,
Thomas
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] vhost-user-test: fix a memory leak
2019-12-11 7:57 ` Thomas Huth
@ 2019-12-11 8:18 ` Marc-André Lureau
2019-12-12 1:48 ` Pan Nengyuan
2019-12-12 1:41 ` Pan Nengyuan
1 sibling, 1 reply; 7+ messages in thread
From: Marc-André Lureau @ 2019-12-11 8:18 UTC (permalink / raw)
To: Thomas Huth
Cc: Laurent Vivier, Paolo Bonzini, pannengyuan, zhanghailiang, QEMU
Hi
On Wed, Dec 11, 2019 at 11:57 AM Thomas Huth <thuth@redhat.com> wrote:
>
> Hi!
>
> On 11/12/2019 01.55, pannengyuan@huawei.com wrote:
> [...]
> > diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
> > index 91ea373..54be931 100644
> > --- a/tests/vhost-user-test.c
> > +++ b/tests/vhost-user-test.c
> > @@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
> > guint64 size;
> >
> > if (!wait_for_fds(s)) {
> > + g_free(uri);
> > + test_server_free(dest);
> > return;
> > }
>
> Well spotted. But I'd prefer to rather move the allocation of these
> resources after the if-statement instead of doing the allocation at the
> declaration of the variables already. Or maybe use a "goto out" and jump
> to the end of the function instead? ... whatever you prefer, but
> duplicating the "free" functions sounds like a cumbersome solution to me.
g_auto (preferably) should work as well.
--
Marc-André Lureau
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] vhost-user-test: fix a memory leak
2019-12-11 8:18 ` Marc-André Lureau
@ 2019-12-12 1:48 ` Pan Nengyuan
0 siblings, 0 replies; 7+ messages in thread
From: Pan Nengyuan @ 2019-12-12 1:48 UTC (permalink / raw)
To: Marc-André Lureau, Thomas Huth
Cc: Laurent Vivier, Paolo Bonzini, zhanghailiang, QEMU
On 2019/12/11 16:18, Marc-André Lureau wrote:
> Hi
>
> On Wed, Dec 11, 2019 at 11:57 AM Thomas Huth <thuth@redhat.com> wrote:
>>
>> Hi!
>>
>> On 11/12/2019 01.55, pannengyuan@huawei.com wrote:
>> [...]
>>> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
>>> index 91ea373..54be931 100644
>>> --- a/tests/vhost-user-test.c
>>> +++ b/tests/vhost-user-test.c
>>> @@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
>>> guint64 size;
>>>
>>> if (!wait_for_fds(s)) {
>>> + g_free(uri);
>>> + test_server_free(dest);
>>> return;
>>> }
>>
>> Well spotted. But I'd prefer to rather move the allocation of these
>> resources after the if-statement instead of doing the allocation at the
>> declaration of the variables already. Or maybe use a "goto out" and jump
>> to the end of the function instead? ... whatever you prefer, but
>> duplicating the "free" functions sounds like a cumbersome solution to me.
>
> g_auto (preferably) should work as well.
Yes, it should work as well. But try to keep it as it is, I will use a
"goto" instead.
Thanks.
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] vhost-user-test: fix a memory leak
2019-12-11 7:57 ` Thomas Huth
2019-12-11 8:18 ` Marc-André Lureau
@ 2019-12-12 1:41 ` Pan Nengyuan
1 sibling, 0 replies; 7+ messages in thread
From: Pan Nengyuan @ 2019-12-12 1:41 UTC (permalink / raw)
To: Thomas Huth, lvivier, pbonzini; +Cc: zhang.zhanghailiang, qemu-devel
On 2019/12/11 15:57, Thomas Huth wrote:
> Hi!
>
> On 11/12/2019 01.55, pannengyuan@huawei.com wrote:
> [...]
>> diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
>> index 91ea373..54be931 100644
>> --- a/tests/vhost-user-test.c
>> +++ b/tests/vhost-user-test.c
>> @@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, QGuestAllocator *alloc)
>> guint64 size;
>>
>> if (!wait_for_fds(s)) {
>> + g_free(uri);
>> + test_server_free(dest);
>> return;
>> }
>
> Well spotted. But I'd prefer to rather move the allocation of these
> resources after the if-statement instead of doing the allocation at the
> declaration of the variables already. Or maybe use a "goto out" and jump
> to the end of the function instead? ... whatever you prefer, but
> duplicating the "free" functions sounds like a cumbersome solution to me.
>
Yes, I think use a "goto out" is more better. I will change it.
Thanks.
> Thanks,
> Thomas
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-12 1:50 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-12-11 0:55 [PATCH] vhost-user-test: fix a memory leak pannengyuan
2019-12-11 7:48 ` Laurent Vivier
2019-12-12 1:11 ` Pan Nengyuan
2019-12-11 7:57 ` Thomas Huth
2019-12-11 8:18 ` Marc-André Lureau
2019-12-12 1:48 ` Pan Nengyuan
2019-12-12 1:41 ` Pan Nengyuan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).