screenshot: don't unlink bogus file

The previous qemu patch could end up calling unlink(tmp) before
tmp was the name of a valid file (unlinking a fileXXXXXX template
instead), or calling unlink(tmp) twice on success (once here,
and once at the end of the stream).  Meanwhile, vbox also suffered
from the same leaked tmp file bug.

* src/qemu/qemu_driver.c (qemuDomainScreenshot): Don't unlink on
success, or on invalid name.
* src/vbox/vbox_tmpl.c (vboxDomainScreenshot): Don't leak temp file.

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5e2c903f6077ee562e0e50a29a87d00f374f233e..02971592d65bdeab2f2ed15df07dd52abb787c4c 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2912,18 +2912,21 @@ qemuDomainScreenshot(virDomainPtr dom,
     qemuDomainObjEnterMonitor(driver, vm);
     if (qemuMonitorScreendump(priv->mon, tmp) < 0) {
         qemuDomainObjExitMonitor(driver, vm);
+        unlink(tmp);
         goto endjob;
     }
     qemuDomainObjExitMonitor(driver, vm);
 
     if (VIR_CLOSE(tmp_fd) < 0) {
         virReportSystemError(errno, _("unable to close %s"), tmp);
+        unlink(tmp);
         goto endjob;
     }
 
     if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY, true) < 0) {
         qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
                         _("unable to open stream"));
+        unlink(tmp);
         goto endjob;
     }
 
@@ -2931,10 +2934,7 @@ qemuDomainScreenshot(virDomainPtr dom,
 
 endjob:
     VIR_FORCE_CLOSE(tmp_fd);
-    if (tmp) {
-        unlink(tmp);
-        VIR_FREE(tmp);
-    }
+    VIR_FREE(tmp);
 
     if (qemuDomainObjEndJob(driver, vm) == 0)
         vm = NULL;

diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a7d354e351c9823553fa51e2961c96e42c3ef40a..66a0fe9365c13ea4eac613ef4adc4f47c36271bf 100644 (file)
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -8713,6 +8713,7 @@ vboxDomainScreenshot(virDomainPtr dom,
                 if (NS_FAILED(rc) || !width || !height) {
                     vboxError(VIR_ERR_OPERATION_FAILED, "%s",
                               _("unable to get screen resolution"));
+                    unlink(tmp);
                     goto endjob;
                 }
 
@@ -8723,6 +8724,7 @@ vboxDomainScreenshot(virDomainPtr dom,
                 if (NS_FAILED(rc)) {
                     vboxError(VIR_ERR_OPERATION_FAILED, "%s",
                               _("failed to take screenshot"));
+                    unlink(tmp);
                     goto endjob;
                 }
 
@@ -8730,17 +8732,20 @@ vboxDomainScreenshot(virDomainPtr dom,
                               screenDataSize) < 0) {
                     virReportSystemError(errno, _("unable to write data "
                                                   "to '%s'"), tmp);
+                    unlink(tmp);
                     goto endjob;
                 }
 
                 if (VIR_CLOSE(tmp_fd) < 0) {
                     virReportSystemError(errno, _("unable to close %s"), tmp);
+                    unlink(tmp);
                     goto endjob;
                 }
 
                 if (virFDStreamOpenFile(st, tmp, 0, 0, O_RDONLY, true) < 0) {
                     vboxError(VIR_ERR_OPERATION_FAILED, "%s",
                               _("unable to open stream"));
+                    unlink(tmp);
                     goto endjob;
                 }