At a lot of places we have the following pattern:

  virXXXDef *def = parseDef();

  if (!(obj = virXXXObjListAdd(def)))
    goto clenaup;
  def = NULL;

 cleanup:
  virXXXDefFree(def);


The 'def = NULL' step is necessary because the ownership of the
definition was transferred onto the object. Well, this approach is
fragile as it relies on developers remembering to set the variable
explicitly.

If the virXXXObjListAdd() would take address of @def then the explicit
set to NULL can be left out.

Please note, I've reworked only a few virXXXObjListAdd() functions to
see whether these are desired or not. If merged, I can post patches for
the rest.

Michal Prívozník (4):
  virInterfaceObjListAssignDef: Transfer definition ownership
  virSecretObjListAdd: Transfer definition ownership
  virStoragePoolObjListAdd: Transfer definition ownership
  virDomainObjListAdd: Transfer definition ownership

 src/bhyve/bhyve_driver.c     |  6 ++----
 src/ch/ch_driver.c           |  7 ++-----
 src/conf/domain_conf.c       |  8 ++++----
 src/conf/domain_conf.h       |  2 +-
 src/conf/virdomainobjlist.c  | 22 +++++++++++++---------
 src/conf/virdomainobjlist.h  |  2 +-
 src/conf/virinterfaceobj.c   | 25 +++++++++++++++++++------
 src/conf/virinterfaceobj.h   |  2 +-
 src/conf/virsecretobj.c      | 26 ++++++++++++++------------
 src/conf/virsecretobj.h      |  2 +-
 src/conf/virstorageobj.c     | 29 ++++++++++++++++-------------
 src/conf/virstorageobj.h     |  2 +-
 src/libxl/libxl_domain.c     |  3 +--
 src/libxl/libxl_driver.c     | 30 ++++++++++--------------------
 src/libxl/libxl_migration.c  |  6 ++----
 src/lxc/lxc_driver.c         | 24 ++++++++----------------
 src/openvz/openvz_conf.c     |  3 +--
 src/openvz/openvz_driver.c   | 10 ++++------
 src/qemu/qemu_driver.c       | 30 ++++++++++--------------------
 src/qemu/qemu_migration.c    |  3 +--
 src/qemu/qemu_snapshot.c     |  9 +++------
 src/secret/secret_driver.c   |  4 ++--
 src/storage/storage_driver.c |  5 ++---
 src/test/test_driver.c       | 31 ++++++++++++-------------------
 src/vmware/vmware_conf.c     |  6 ++----
 src/vmware/vmware_driver.c   | 10 ++++------
 26 files changed, 137 insertions(+), 170 deletions(-)

-- 
2.32.0

On Tue, Nov 23, 2021 at 06:09:36PM +0100, Michal Privoznik wrote:
>At a lot of places we have the following pattern:
>
>  virXXXDef *def = parseDef();
>
>  if (!(obj = virXXXObjListAdd(def)))
>    goto clenaup;
>  def = NULL;
>
> cleanup:
>  virXXXDefFree(def);
>
>
>The 'def = NULL' step is necessary because the ownership of the
>definition was transferred onto the object. Well, this approach is
>fragile as it relies on developers remembering to set the variable
>explicitly.
>
>If the virXXXObjListAdd() would take address of @def then the explicit
>set to NULL can be left out.
>
>Please note, I've reworked only a few virXXXObjListAdd() functions to
>see whether these are desired or not. If merged, I can post patches for
>the rest.
>
>Michal Prívozník (4):
>  virInterfaceObjListAssignDef: Transfer definition ownership
>  virSecretObjListAdd: Transfer definition ownership
>  virStoragePoolObjListAdd: Transfer definition ownership
>  virDomainObjListAdd: Transfer definition ownership

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>