Re: [PATCH 15/18] conf: use disk source accessors in vmx/

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/21/2014 03:10 PM, Laine Stump wrote:
> On 03/19/2014 11:20 AM, Eric Blake wrote:
>> Part of a series of cleanups to use new accessor methods.
>>
>> * src/vmx/vmx.c (virVMXHandleLegacySCSIDiskDriverName)
>> (virVMXParseDisk, virVMXFormatDisk, virVMXFormatFloppy): Use
>> accessors.

>>
>> -            (*def)->type = VIR_DOMAIN_DISK_TYPE_FILE;
>> -            (*def)->src = ctx->parseFileName(fileName, ctx->opaque);
>> +            virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
>> +            if (virDomainDiskSetSource(*def,
>> +                                       ctx->parseFileName(fileName,
>> +                                                          ctx->opaque)) < 0)
> 
> 
> The above will leak the string returned from ctx->parseFileName. There
> is also at least one other place further down where the same thing occurs.

Good catch.  Here's what I'm squashing to fix it.

diff --git i/src/vmx/vmx.c w/src/vmx/vmx.c
index 341d1af..9b5162b 100644
--- i/src/vmx/vmx.c
+++ w/src/vmx/vmx.c
@@ -2164,6 +2164,8 @@ virVMXParseDisk(virVMXContext *ctx,
virDomainXMLOptionPtr xmlopt, virConfPtr con
     /* Setup virDomainDiskDef */
     if (device == VIR_DOMAIN_DISK_DEVICE_DISK) {
         if (virFileHasSuffix(fileName, ".vmdk")) {
+            char *tmp;
+
             if (deviceType != NULL) {
                 if (busType == VIR_DOMAIN_DISK_BUS_SCSI &&
                     STRCASENEQ(deviceType, "scsi-hardDisk") &&
@@ -2185,19 +2187,18 @@ virVMXParseDisk(virVMXContext *ctx,
virDomainXMLOptionPtr xmlopt, virConfPtr con
             }

             virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
-            if (virDomainDiskSetSource(*def,
-                                       ctx->parseFileName(fileName,
-                                                          ctx->opaque))
< 0)
+            if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
+                goto cleanup;
+            if (virDomainDiskSetSource(*def, tmp) < 0) {
+                VIR_FREE(tmp);
                 goto cleanup;
+            }
+            VIR_FREE(tmp);
             (*def)->cachemode = writeThrough ?
VIR_DOMAIN_DISK_CACHE_WRITETHRU
                                              :
VIR_DOMAIN_DISK_CACHE_DEFAULT;
             if (mode)
                 (*def)->transient = STRCASEEQ(mode,
                                               "independent-nonpersistent");
-
-            if (!virDomainDiskGetSource(*def)) {
-                goto cleanup;
-            }
         } else if (virFileHasSuffix(fileName, ".iso") ||
                    STRCASEEQ(deviceType, "atapi-cdrom") ||
                    STRCASEEQ(deviceType, "cdrom-raw")) {
@@ -2218,6 +2219,8 @@ virVMXParseDisk(virVMXContext *ctx,
virDomainXMLOptionPtr xmlopt, virConfPtr con
         }
     } else if (device == VIR_DOMAIN_DISK_DEVICE_CDROM) {
         if (virFileHasSuffix(fileName, ".iso")) {
+            char *tmp;
+
             if (deviceType != NULL) {
                 if (STRCASENEQ(deviceType, "cdrom-image")) {
                     virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -2228,14 +2231,13 @@ virVMXParseDisk(virVMXContext *ctx,
virDomainXMLOptionPtr xmlopt, virConfPtr con
             }

             virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
-            if (virDomainDiskSetSource(*def,
-                                       ctx->parseFileName(fileName,
-                                                          ctx->opaque))
< 0)
+            if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
                 goto cleanup;
-
-            if (!virDomainDiskGetSource(*def)) {
+            if (virDomainDiskSetSource(*def, tmp) < 0) {
+                VIR_FREE(tmp);
                 goto cleanup;
             }
+            VIR_FREE(tmp);
         } else if (virFileHasSuffix(fileName, ".vmdk")) {
             /*
              * This function was called in order to parse a CDROM
device, but
@@ -2278,11 +2280,16 @@ virVMXParseDisk(virVMXContext *ctx,
virDomainXMLOptionPtr xmlopt, virConfPtr con
             if (virDomainDiskSetSource(*def, fileName) < 0)
                 goto cleanup;
         } else if (fileType != NULL && STRCASEEQ(fileType, "file")) {
+            char *tmp;
+
             virDomainDiskSetType(*def, VIR_DOMAIN_DISK_TYPE_FILE);
-            if (virDomainDiskSetSource(*def,
-                                       ctx->parseFileName(fileName,
-                                                          ctx->opaque))
< 0)
+            if (!(tmp = ctx->parseFileName(fileName, ctx->opaque)))
                 goto cleanup;
+            if (virDomainDiskSetSource(*def, tmp) < 0) {
+                VIR_FREE(tmp);
+                goto cleanup;
+            }
+            VIR_FREE(tmp);
         } else {
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Invalid or not yet handled value '%s' "


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]