Re: [libvirt] qemuDomainSnapshotLoad leak
On 04/07/2010 11:47 AM, Jim Meyering wrote:
clang reports that the assignment to "snap" below is a dead
store.
Actually it's also a leak, and it seems like it deserves a diagnostic
if/when that function returns NULL.
It is a dead store, that is true, but it's not a leak. The pointer that
is returned there is a pointer to the internal snapshot object, which
we want to keep around. I guess we can just do:
virDomainSnapshotAssignDef(&vm->snapshots, def);
And don't do the assignment at all. And yes, we should also check for
NULL and print an error if it fails.
It looks to me like this function should return
something other than "void", so that it can inform
its caller of such a failure.
No?
...but no, we should leave this function returning void. If something (anything)
fails in here, there is nothing a higher level can do. We don't want to fail
to start libvirtd because of one corrupt/bogus snapshot metadata file,
so the most we can do is print an error message and go on.
--
Chris Lalancette