Re: [Qemu-devel] [PATCH] migration: fix small leaks

[Peter Xu]

On Wed, Dec 27, 2017 at 03:25:23PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Hmm, looks like leak is not fixed here: I've checked it while running iotest
> 181, that
> migration_instance_finalize is not called.
> 
> If I understand correct, to call it we need unref current_migration object
> somewhere.
> 
> Or, may be I'm missing something..

I think you are right.

It does not matter much though since we don't dynamically allocate
migration object (there is only one and it lives forever).  Do you
want to post a patch?  I guess the safest place to unref it is at the
end of main() to make sure no one will be using it any more.

(Hmm, the incoming migration state is still static)

Thanks,

> 
> 01.08.2017 19:04, Marc-André Lureau wrote:
> > Spotted thanks to valgrind and tests/device-introspect-test:
> > 
> > ==11711== 1 bytes in 1 blocks are definitely lost in loss record 6 of 14,537
> > ==11711==    at 0x4C2EB6B: malloc (vg_replace_malloc.c:299)
> > ==11711==    by 0x1E0CDBD8: g_malloc (gmem.c:94)
> > ==11711==    by 0x1E0E696E: g_strdup (gstrfuncs.c:363)
> > ==11711==    by 0x695693: migration_instance_init (migration.c:2226)
> > ==11711==    by 0x717C4B: object_init_with_type (object.c:344)
> > ==11711==    by 0x717E80: object_initialize_with_type (object.c:375)
> > ==11711==    by 0x7182EB: object_new_with_type (object.c:483)
> > ==11711==    by 0x718328: object_new (object.c:493)
> > ==11711==    by 0x4B8A29: qmp_device_list_properties (qmp.c:542)
> > ==11711==    by 0x4A9561: qmp_marshal_device_list_properties 
> > (qmp-marshal.c:1425)
> > ==11711==    by 0x819D4A: do_qmp_dispatch (qmp-dispatch.c:104)
> > ==11711==    by 0x819E82: qmp_dispatch (qmp-dispatch.c:131)
> > 
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > ---
> >   migration/migration.c | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> > 
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 085c32c994..c3fe0ed9ca 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -2214,6 +2214,15 @@ static void migration_class_init(ObjectClass *klass, 
> > void *data)
> >       dc->props = migration_properties;
> >   }
> > +static void migration_instance_finalize(Object *obj)
> > +{
> > +    MigrationState *ms = MIGRATION_OBJ(obj);
> > +    MigrationParameters *params = &ms->parameters;
> > +
> > +    g_free(params->tls_hostname);
> > +    g_free(params->tls_creds);
> > +}
> > +
> >   static void migration_instance_init(Object *obj)
> >   {
> >       MigrationState *ms = MIGRATION_OBJ(obj);
> > @@ -2282,6 +2291,7 @@ static const TypeInfo migration_type = {
> >       .class_size = sizeof(MigrationClass),
> >       .instance_size = sizeof(MigrationState),
> >       .instance_init = migration_instance_init,
> > +    .instance_finalize = migration_instance_finalize,
> >   };
> >   static void register_migration_types(void)
> 
> 
> -- 
> Best regards,
> Vladimir
> 
> 

-- 
Peter Xu