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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list