lib: Undo some g_steal_pointer() changes

Recently, a few commits back I've switched bunch of code to
g_steal_pointer() using coccinelle. Problem was that the semantic
patch used was slightly off:

  @@
  expression a, b;
  @@

  + b = g_steal_pointer(&a);
  - b = a;
    ... when != a
  - a = NULL;

Problem is that, "... when != a" is supposed to jump over those
lines, which don't contain expression a. My idea was to replace
the following pattern too:

  ptrX = ptrY;
  if (something(ptrZ) < 0) goto error;
  ptrY = NULL;

But what I missed is that the following pattern is also matched
and replaced:

  ptrX = ptrY;
  if (something(ptrX) < 0) goto error;
  ptrY = NULL;

This is not necessarily correct - as demonstrated by our hotplug
code. The real problem is ambiguous memory ownership transfer
(functions which add device to domain def take ownership only on
success), but to not tackle the real issue let's revert those
parts.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 0928d2756302f4c210046fdbb3a6dfafd868c195..7f43625638a81f5a7fdbf82b647364e97048b74f 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -3529,7 +3529,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
 
     switch (dev->type) {
         case VIR_DOMAIN_DEVICE_DISK:
-            disk = g_steal_pointer(&dev->data.disk);
+            disk = dev->data.disk;
             if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) {
                 virReportError(VIR_ERR_INVALID_ARG,
                                _("target %s already exists."), disk->dst);
@@ -3537,10 +3537,11 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
             }
             virDomainDiskInsert(vmdef, disk);
             /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
+            dev->data.disk = NULL;
             break;
 
         case VIR_DOMAIN_DEVICE_CONTROLLER:
-            controller = g_steal_pointer(&dev->data.controller);
+            controller = dev->data.controller;
             if (controller->idx != -1 &&
                 virDomainControllerFind(vmdef, controller->type,
                                         controller->idx) >= 0) {
@@ -3550,10 +3551,11 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
             }
 
             virDomainControllerInsert(vmdef, controller);
+            dev->data.controller = NULL;
             break;
 
         case VIR_DOMAIN_DEVICE_NET:
-            net = g_steal_pointer(&dev->data.net);
+            net = dev->data.net;
             if (virDomainHasNet(vmdef, net)) {
                 virReportError(VIR_ERR_INVALID_ARG,
                                _("network device with mac %s already exists"),
@@ -3562,6 +3564,7 @@ libxlDomainAttachDeviceConfig(virDomainDefPtr vmdef, virDomainDeviceDefPtr dev)
             }
             if (virDomainNetInsert(vmdef, net))
                 return -1;
+            dev->data.net = NULL;
             break;
 
         case VIR_DOMAIN_DEVICE_HOSTDEV:

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c613019c7f50b00395c104e8186a5215dd3f6c16..2db544c94515c8ea01a09b02b14be6e1a393b10f 100644 (file)
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -3042,7 +3042,7 @@ lxcDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
-        disk = g_steal_pointer(&dev->data.disk);
+        disk = dev->data.disk;
         if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) {
             virReportError(VIR_ERR_INVALID_ARG,
                            _("target %s already exists."), disk->dst);
@@ -3050,18 +3050,20 @@ lxcDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         }
         virDomainDiskInsert(vmdef, disk);
         /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
+        dev->data.disk = NULL;
         ret = 0;
         break;
 
     case VIR_DOMAIN_DEVICE_NET:
-        net = g_steal_pointer(&dev->data.net);
+        net = dev->data.net;
         if (virDomainNetInsert(vmdef, net) < 0)
             return -1;
+        dev->data.net = NULL;
         ret = 0;
         break;
 
     case VIR_DOMAIN_DEVICE_HOSTDEV:
-        hostdev = g_steal_pointer(&dev->data.hostdev);
+        hostdev = dev->data.hostdev;
         if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) {
             virReportError(VIR_ERR_INVALID_ARG, "%s",
                            _("device is already in the domain configuration"));
@@ -3069,6 +3071,7 @@ lxcDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         }
         if (virDomainHostdevInsert(vmdef, hostdev) < 0)
             return -1;
+        dev->data.hostdev = NULL;
         ret = 0;
         break;
 
@@ -3093,7 +3096,7 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
     switch (dev->type) {
     case VIR_DOMAIN_DEVICE_NET:
-        net = g_steal_pointer(&dev->data.net);
+        net = dev->data.net;
         if ((idx = virDomainNetFindIdx(vmdef, net)) < 0)
             return -1;
 
@@ -3107,6 +3110,7 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
             return -1;
 
         virDomainNetDefFree(oldDev.data.net);
+        dev->data.net = NULL;
         ret = 0;
 
         break;

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3e9fc88ca15f98d012e665014e26129d222e4f26..69dc704a44b44b890cefccf234ebaebcb35777a5 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7239,7 +7239,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 
     switch ((virDomainDeviceType)dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
-        disk = g_steal_pointer(&dev->data.disk);
+        disk = dev->data.disk;
         if (virDomainDiskIndexByName(vmdef, disk->dst, true) >= 0) {
             virReportError(VIR_ERR_OPERATION_INVALID,
                            _("target %s already exists"), disk->dst);
@@ -7251,22 +7251,25 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
             return -1;
         virDomainDiskInsert(vmdef, disk);
         /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
+        dev->data.disk = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_NET:
-        net = g_steal_pointer(&dev->data.net);
+        net = dev->data.net;
         if (virDomainNetInsert(vmdef, net))
             return -1;
+        dev->data.net = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_SOUND:
-        sound = g_steal_pointer(&dev->data.sound);
+        sound = dev->data.sound;
         if (VIR_APPEND_ELEMENT(vmdef->sounds, vmdef->nsounds, sound) < 0)
             return -1;
+        dev->data.sound = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_HOSTDEV:
-        hostdev = g_steal_pointer(&dev->data.hostdev);
+        hostdev = dev->data.hostdev;
         if (virDomainHostdevFind(vmdef, hostdev, NULL) >= 0) {
             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                            _("device is already in the domain configuration"));
@@ -7274,10 +7277,11 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         }
         if (virDomainHostdevInsert(vmdef, hostdev))
             return -1;
+        dev->data.hostdev = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_LEASE:
-        lease = g_steal_pointer(&dev->data.lease);
+        lease = dev->data.lease;
         if (virDomainLeaseIndex(vmdef, lease) >= 0) {
             virReportError(VIR_ERR_OPERATION_INVALID,
                            _("Lease %s in lockspace %s already exists"),
@@ -7287,10 +7291,11 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         virDomainLeaseInsert(vmdef, lease);
 
         /* vmdef has the pointer. Generic codes for vmdef will do all jobs */
+        dev->data.lease = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_CONTROLLER:
-        controller = g_steal_pointer(&dev->data.controller);
+        controller = dev->data.controller;
         if (controller->idx != -1 &&
             virDomainControllerFind(vmdef, controller->type,
                                     controller->idx) >= 0) {
@@ -7301,6 +7306,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         }
 
         virDomainControllerInsert(vmdef, controller);
+        dev->data.controller = NULL;
 
         break;
 
@@ -7311,7 +7317,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         break;
 
     case VIR_DOMAIN_DEVICE_FS:
-        fs = g_steal_pointer(&dev->data.fs);
+        fs = dev->data.fs;
         if (virDomainFSIndexByName(vmdef, fs->dst) >= 0) {
             virReportError(VIR_ERR_OPERATION_INVALID,
                          "%s", _("Target already exists"));
@@ -7320,6 +7326,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
 
         if (virDomainFSInsert(vmdef, fs) < 0)
             return -1;
+        dev->data.fs = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_RNG:
@@ -7351,14 +7358,15 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         break;
 
     case VIR_DOMAIN_DEVICE_REDIRDEV:
-        redirdev = g_steal_pointer(&dev->data.redirdev);
+        redirdev = dev->data.redirdev;
 
         if (VIR_APPEND_ELEMENT(vmdef->redirdevs, vmdef->nredirdevs, redirdev) < 0)
             return -1;
+        dev->data.redirdev = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_SHMEM:
-        shmem = g_steal_pointer(&dev->data.shmem);
+        shmem = dev->data.shmem;
         if (virDomainShmemDefFind(vmdef, shmem) >= 0) {
             virReportError(VIR_ERR_OPERATION_INVALID, "%s",
                            _("device is already in the domain configuration"));
@@ -7366,6 +7374,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
         }
         if (virDomainShmemDefInsert(vmdef, shmem) < 0)
             return -1;
+        dev->data.shmem = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_WATCHDOG:
@@ -7637,7 +7646,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
     switch ((virDomainDeviceType)dev->type) {
     case VIR_DOMAIN_DEVICE_DISK:
-        newDisk = g_steal_pointer(&dev->data.disk);
+        newDisk = dev->data.disk;
         if ((pos = virDomainDiskIndexByName(vmdef, newDisk->dst, false)) < 0) {
             virReportError(VIR_ERR_INVALID_ARG,
                            _("target %s doesn't exist."), newDisk->dst);
@@ -7652,10 +7661,11 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
         virDomainDiskDefFree(vmdef->disks[pos]);
         vmdef->disks[pos] = newDisk;
+        dev->data.disk = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_GRAPHICS:
-        newGraphics = g_steal_pointer(&dev->data.graphics);
+        newGraphics = dev->data.graphics;
         pos = qemuDomainFindGraphicsIndex(vmdef, newGraphics);
         if (pos < 0) {
             virReportError(VIR_ERR_INVALID_ARG,
@@ -7672,10 +7682,11 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
 
         virDomainGraphicsDefFree(vmdef->graphics[pos]);
         vmdef->graphics[pos] = newGraphics;
+        dev->data.graphics = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_NET:
-        net = g_steal_pointer(&dev->data.net);
+        net = dev->data.net;
         if ((pos = virDomainNetFindIdx(vmdef, net)) < 0)
             return -1;
 
@@ -7689,6 +7700,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
             return -1;
 
         virDomainNetDefFree(oldDev.data.net);
+        dev->data.net = NULL;
         break;
 
     case VIR_DOMAIN_DEVICE_FS: