tests/vhost-user-test.c | 2 ++ 1 file changed, 2 insertions(+)
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
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
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 > >
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
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
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. > >
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 > >
© 2016 - 2024 Red Hat, Inc.