On Wed, Apr 20, 2022 at 12:13:07PM +0100, Daniel P. Berrangé wrote: > On Thu, Mar 31, 2022 at 11:08:53AM -0400, Peter Xu wrote: > > It's useful for specifying tls credentials all in the cmdline (along with > > the -object tls-creds-*), especially for debugging purpose. > > > > The trick here is we must remember to not free these fields again in the > > finalize() function of migration object, otherwise it'll cause double-free. > > > > The thing is when destroying an object, we'll first destroy the properties > > that bound to the object, then the object itself. To be explicit, when > > destroy the object in object_finalize() we have such sequence of > > operations: > > > > object_property_del_all(obj); > > object_deinit(obj, ti); > > > > So after this change the two fields are properly released already even > > before reaching the finalize() function but in object_property_del_all(), > > hence we don't need to free them anymore in finalize() or it's double-free. > > > I believe this is also fixing a small memory leak
Yes I think so. I didn't even mention it since it's one global tiny variable and IIUC QEMU does have other similar cases of keeping vars around. As long as it'll not grow dynamically, then doesn't sound like a huge problem. But yeah, doing proper free is still ideal. So I'll add one more sentence to the commit message in next version. Thanks, -- Peter Xu
