When virAsprintf was changed from a function to a macro reporting OOM error in dc6f2da, it was documented as returning 0 on success. This is incorrect, it returns the number of bytes written as asprintf does. Some of the functions were converted to use virAsprintf's return value directly, changing the return value on success from 0 to >= 0. For most of these, this is not a problem, but the change in virPCIDriverDir breaks PCI passthrough. The return value check in virhashtest pre-dates virAsprintf OOM conversion. vmwareMakePath seems to be unused. --- src/util/virstring.h | 10 ++++++---- src/util/virpci.c | 26 ++++++++++++++++++-------- tests/virhashtest.c | 2 +- src/qemu/qemu_command.c | 13 ++++++++++--- src/qemu/qemu_process.c | 6 ++++-- src/util/virnetdev.c | 10 +++++++--- src/util/virrandom.c | 6 ++++-- src/vmware/vmware_conf.c | 15 ++++++++++++--- 8 files changed, 62 insertions(+), 26 deletions(-) diff --git a/src/util/virstring.h b/src/util/virstring.h index 8b66b23..b390150 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -176,7 +176,8 @@ size_t virStringListLength(char **strings); * Like glibc's vasprintf but makes sure *strp == NULL on failure, in which * case the OOM error is reported too. * - * Returns -1 on failure (with OOM error reported), 0 on success. + * Returns -1 on failure (with OOM error reported), number of bytes printed + * on success. */ # define virVasprintf(strp, fmt, list) \ virVasprintfInternal(true, VIR_FROM_THIS, __FILE__, __FUNCTION__, \ @@ -187,7 +188,7 @@ size_t virStringListLength(char **strings); * * Like glibc's vasprintf but makes sure *strp == NULL on failure. * - * Returns -1 on failure, 0 on success. + * Returns -1 on failure, number of bytes printed on success. */ # define virVasprintfQuiet(strp, fmt, list) \ virVasprintfInternal(false, 0, NULL, NULL, 0, strp, fmt, list) @@ -200,7 +201,8 @@ size_t virStringListLength(char **strings); * Like glibc's_asprintf but makes sure *strp == NULL on failure, in which case * the OOM error is reported too. * - * Returns -1 on failure (with OOM error reported), 0 on success. + * Returns -1 on failure (with OOM error reported), number of bytes printed + * on success. */ # define virAsprintf(strp, ...) \ @@ -214,7 +216,7 @@ size_t virStringListLength(char **strings); * * Like glibc's_asprintf but makes sure *strp == NULL on failure. * - * Returns -1 on failure, 0 on success. + * Returns -1 on failure, number of bytes printed on success. */ # define virAsprintfQuiet(strp, ...) \ diff --git a/src/util/virpci.c b/src/util/virpci.c index c95c0e6..be50b4f 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -194,7 +194,9 @@ virPCIDriverDir(char **buffer, const char *driver) { VIR_FREE(*buffer); - return virAsprintf(buffer, PCI_SYSFS "drivers/%s", driver); + if (virAsprintf(buffer, PCI_SYSFS "drivers/%s", driver) < 0) + return -1; + return 0; } @@ -203,7 +205,9 @@ virPCIDriverFile(char **buffer, const char *driver, const char *file) { VIR_FREE(*buffer); - return virAsprintf(buffer, PCI_SYSFS "drivers/%s/%s", driver, file); + if (virAsprintf(buffer, PCI_SYSFS "drivers/%s/%s", driver, file) < 0) + return -1; + return 0; } @@ -212,7 +216,9 @@ virPCIFile(char **buffer, const char *device, const char *file) { VIR_FREE(*buffer); - return virAsprintf(buffer, PCI_SYSFS "devices/%s/%s", device, file); + if (virAsprintf(buffer, PCI_SYSFS "devices/%s/%s", device, file) < 0) + return -1; + return 0; } @@ -2529,17 +2535,21 @@ out: int virPCIGetSysfsFile(char *virPCIDeviceName, char **pci_sysfs_device_link) { - return virAsprintf(pci_sysfs_device_link, PCI_SYSFS "devices/%s", - virPCIDeviceName); + if (virAsprintf(pci_sysfs_device_link, PCI_SYSFS "devices/%s", + virPCIDeviceName) < 0) + return -1; + return 0; } int virPCIDeviceAddressGetSysfsFile(virPCIDeviceAddressPtr dev, char **pci_sysfs_device_link) { - return virAsprintf(pci_sysfs_device_link, - PCI_SYSFS "devices/%04x:%02x:%02x.%x", dev->domain, - dev->bus, dev->slot, dev->function); + if (virAsprintf(pci_sysfs_device_link, + PCI_SYSFS "devices/%04x:%02x:%02x.%x", dev->domain, + dev->bus, dev->slot, dev->function) < 0) + return -1; + return 0; } /* diff --git a/tests/virhashtest.c b/tests/virhashtest.c index 2430432..dd2c948 100644 --- a/tests/virhashtest.c +++ b/tests/virhashtest.c @@ -18,7 +18,7 @@ #define testError(...) \ do { \ char *str; \ - if (virAsprintfQuiet(&str, __VA_ARGS__) == 0) { \ + if (virAsprintfQuiet(&str, __VA_ARGS__) >= 0) { \ fprintf(stderr, "%s", str); \ VIR_FREE(str); \ } \ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 71e37f3..38b224b 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -796,7 +796,9 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def, virDomainNetDefPtr net, int idx) } } - return virAsprintf(&net->info.alias, "net%d", idx); + if (virAsprintf(&net->info.alias, "net%d", idx) < 0) + return -1; + return 0; } @@ -851,7 +853,9 @@ qemuAssignDeviceRedirdevAlias(virDomainDefPtr def, virDomainRedirdevDefPtr redir } } - return virAsprintf(&redirdev->info.alias, "redir%d", idx); + if (virAsprintf(&redirdev->info.alias, "redir%d", idx) < 0) + return -1; + return 0; } @@ -860,7 +864,10 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller) { const char *prefix = virDomainControllerTypeToString(controller->type); - return virAsprintf(&controller->info.alias, "%s%d", prefix, controller->idx); + if (virAsprintf(&controller->info.alias, "%s%d", prefix, + controller->idx) < 0) + return -1; + return 0; } static ssize_t diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 3d5e8f6..73b862d 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2562,8 +2562,10 @@ qemuProcessPrepareMonitorChr(virQEMUDriverConfigPtr cfg, monConfig->type = VIR_DOMAIN_CHR_TYPE_UNIX; monConfig->data.nix.listen = true; - return virAsprintf(&monConfig->data.nix.path, "%s/%s.monitor", - cfg->libDir, vm); + if (virAsprintf(&monConfig->data.nix.path, "%s/%s.monitor", + cfg->libDir, vm) < 0) + return -1; + return 0; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index aaa276d..30df7a6 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1050,7 +1050,9 @@ virNetDevSysfsFile(char **pf_sysfs_device_link, const char *ifname, const char *file) { - return virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file); + if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/%s", ifname, file) < 0) + return -1; + return 0; } static int @@ -1058,8 +1060,10 @@ virNetDevSysfsDeviceFile(char **pf_sysfs_device_link, const char *ifname, const char *file) { - return virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/device/%s", - ifname, file); + if (virAsprintf(pf_sysfs_device_link, NET_SYSFS "%s/device/%s", ifname, + file) < 0) + return -1; + return 0; } /** diff --git a/src/util/virrandom.c b/src/util/virrandom.c index e2d18f8..c233732 100644 --- a/src/util/virrandom.c +++ b/src/util/virrandom.c @@ -178,6 +178,8 @@ virRandomGenerateWWN(char **wwn, return -1; } - return virAsprintf(wwn, "5" "%s%09llx", oui, - (unsigned long long)virRandomBits(36)); + if (virAsprintf(wwn, "5" "%s%09llx", oui, + (unsigned long long)virRandomBits(36)) < 0) + return -1; + return 0; } diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 96f69b3..ed99e59 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -313,9 +313,16 @@ error: int vmwareConstructVmxPath(char *directoryName, char *name, char **vmxPath) { + int ret; + if (directoryName != NULL) - return virAsprintf(vmxPath, "%s/%s.vmx", directoryName, name); - return virAsprintf(vmxPath, "%s.vmx", name); + ret = virAsprintf(vmxPath, "%s/%s.vmx", directoryName, name); + else + ret = virAsprintf(vmxPath, "%s.vmx", name); + + if (ret < 0) + return -1; + return 0; } int @@ -414,7 +421,9 @@ vmwareMoveFile(char *srcFile, char *dstFile) int vmwareMakePath(char *srcDir, char *srcName, char *srcExt, char **outpath) { - return virAsprintf(outpath, "%s/%s.%s", srcDir, srcName, srcExt); + if (virAsprintf(outpath, "%s/%s.%s", srcDir, srcName, srcExt) < 0) + return -1; + return 0; } int -- 1.8.1.5