[libvirt] qemuDomainSnapshotLoad leak

Jim Meyering jim at meyering.net
Wed Apr 7 15:47:11 UTC 2010


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 looks to me like this function should return
something other than "void", so that it can inform
its caller of such a failure.

No?

This is from qemu_driver.c:

  static void qemuDomainSnapshotLoad(void *payload,
                                     const char *name ATTRIBUTE_UNUSED,
                                     void *data)
  {
      ...
      virDomainSnapshotObjPtr snap = NULL;
      ...
      while ((entry = readdir(dir))) {
          ...
          def = virDomainSnapshotDefParseString(xmlStr, 0);
          if (def == NULL) {
              /* Nothing we can do here, skip this one */
              VIR_ERROR("Failed to parse snapshot XML from file '%s'", fullpath);
              VIR_FREE(xmlStr);
              continue;
          }

          snap = virDomainSnapshotAssignDef(&vm->snapshots, def);

          VIR_FREE(xmlStr);
      }

      /* FIXME: qemu keeps internal track of snapshots.  We can get access
       * to this info via the "info snapshots" monitor command for running
       * domains, or via "qemu-img snapshot -l" for shutoff domains.  It would
       * be nice to update our internal state based on that, but there is a
       * a problem.  qemu doesn't track all of the same metadata that we do.
       * In particular we wouldn't be able to fill in the <parent>, which is
       * pretty important in our metadata.
       */

      virResetLastError();

  cleanup:
      if (dir)
          closedir(dir);
      VIR_FREE(snapDir);
      virDomainObjUnlock(vm);
  }




More information about the libvir-list mailing list