tests: fix segfault in objecteventtest

Test 12 from objecteventtest (createXML add event) segaults on FreeBSD
with bus error.

At some point it calls testNodeDeviceDestroy() from the test driver. And
it fails when it tries to unlock the device in the "out:" label of this
function.

Unlocking fails because the previous step was a call to
virNodeDeviceObjRemove from conf/node_device_conf.c. This function
removes the given device from the device list and cleans up the object,
including destroying of its mutex. However, it does not nullify the pointer
that was given to it.

As a result, we end up in testNodeDeviceDestroy() here:

 out:
    if (obj)
        virNodeDeviceObjUnlock(obj);

And instead of skipping this, we try to do Unlock and fail because of
malformed mutex.

Change virNodeDeviceObjRemove to use double pointer and set pointer to
NULL.

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index a23d8ef99ed92f02b3cc8db32c9173fe9e620f33..719abcd21c4666a36a587ee64e6f9b27f99dd561 100644 (file)
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -207,22 +207,23 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
 }
 
 void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                            virNodeDeviceObjPtr dev)
+                            virNodeDeviceObjPtr *dev)
 {
     size_t i;
 
-    virNodeDeviceObjUnlock(dev);
+    virNodeDeviceObjUnlock(*dev);
 
     for (i = 0; i < devs->count; i++) {
-        virNodeDeviceObjLock(dev);
-        if (devs->objs[i] == dev) {
-            virNodeDeviceObjUnlock(dev);
+        virNodeDeviceObjLock(*dev);
+        if (devs->objs[i] == *dev) {
+            virNodeDeviceObjUnlock(*dev);
             virNodeDeviceObjFree(devs->objs[i]);
+            *dev = NULL;
 
             VIR_DELETE_ELEMENT(devs->objs, i, devs->count);
             break;
         }
-        virNodeDeviceObjUnlock(dev);
+        virNodeDeviceObjUnlock(*dev);
     }
 }
 

diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 8f23a986cd97bf87d104055991f74daa6d61ac1a..9f005000242084bb14f7287b2d5b9323cb067a5a 100644 (file)
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h
@@ -249,7 +249,7 @@ virNodeDeviceObjPtr virNodeDeviceAssignDef(virNodeDeviceObjListPtr devs,
                                            virNodeDeviceDefPtr def);
 
 void virNodeDeviceObjRemove(virNodeDeviceObjListPtr devs,
-                            virNodeDeviceObjPtr dev);
+                            virNodeDeviceObjPtr *dev);
 
 char *virNodeDeviceDefFormat(const virNodeDeviceDef *def);
 

diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c
index ef0cd9c1091aeb80c5b619ed0e570e81aa9b9e29..fb7bf255db719b242569ab8118d6a065a3d7bdee 100644 (file)
--- a/src/node_device/node_device_hal.c
+++ b/src/node_device/node_device_hal.c
@@ -530,7 +530,7 @@ dev_refresh(const char *udi)
         /* Simply "rediscover" device -- incrementally handling changes
          * to sub-capabilities (like net.80203) is nasty ... so avoid it.
          */
-        virNodeDeviceObjRemove(&driver->devs, dev);
+        virNodeDeviceObjRemove(&driver->devs, &dev);
     } else {
         VIR_DEBUG("no device named %s", name);
     }
@@ -560,7 +560,7 @@ device_removed(LibHalContext *ctx ATTRIBUTE_UNUSED,
     dev = virNodeDeviceFindByName(&driver->devs, name);
     VIR_DEBUG("%s", name);
     if (dev)
-        virNodeDeviceObjRemove(&driver->devs, dev);
+        virNodeDeviceObjRemove(&driver->devs, &dev);
     else
         VIR_DEBUG("no device named %s", name);
     nodeDeviceUnlock();

diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index ddf3d8868efa26435cfc60b0b00923b6792342a5..520269fbe94c5b12a3f60969c4329cfce3c8db3e 100644 (file)
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1044,7 +1044,7 @@ static int udevRemoveOneDevice(struct udev_device *device)
 
     VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
               dev->def->name, name);
-    virNodeDeviceObjRemove(&driver->devs, dev);
+    virNodeDeviceObjRemove(&driver->devs, &dev);
 
     ret = 0;
  cleanup:

diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index bc1f93d89d7da3791fdfa977902c541934990457..53cfa3cf78c05856bf1ff2e22be7c77313c006e8 100644 (file)
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -5534,7 +5534,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev)
                                            0);
 
     virNodeDeviceObjLock(obj);
-    virNodeDeviceObjRemove(&driver->devs, obj);
+    virNodeDeviceObjRemove(&driver->devs, &obj);
 
  out:
     if (obj)