On 05/20/2013 11:55 AM, Michal Privoznik wrote: silent->noisy, but all callers reported OOM via a no_memory label, and I'm okay with double-oom if you are still planning on making another pass to reduce the number of no_memory labels in the code base. silent->noisy. esx_driver.c and qemu_capabilities aren't checking for error (they should), xen_hypervisor has double-oom via no_memory label, and tests/vmx2xmltest.c and tests/xml2vmxtest.c can get by as-is. silent->noisy, but all callers (outside tests) used no_memory label and now have double-oom. Local double-oom. You might want to clean this one up now. another local double-oom. Cleaning now would be nice, but I guess I can live with the promise of later cleanup of 'no_memory' labels. More double-oom and again Here, you have double-oom, but you also have a VIR_ALLOC_N. I can definitely agree to leaving this one as-is, since we are going to audit VIR_ALLOC_N later and can clean up the double-oom then (the earlier uses have no VIR_ALLOC_N nearby, but if your goal is to get rid of no_memory labels in general, you'd still get them on a later pass). Might look nicer as: if (VIR_STRNDUP(listenInfo->address, address, len == -1 ? strlen(address) : len) < 0) Another site where calling just VIR_STRNDUP looks nicer. double-oom, but mixed with VIR_ALLOC so okay for now silent->noisy, but this is a case where we are trying to send an asynchronous event, and merely discard it on OOM. This change now leaves an error in the thread-local storage, to be used in the next unrelated synchronous command that uses the same worker thread. Are we SURE this is right? That is, if ALL libvirt API clear out the local error on entry, then checking if the local error is set on exit, then littering the thread-local error at other times won't change the behavior of synchronous API. But I think you got lucky: looking at src/libvirt.c, we are pretty consistent on virResetLastError() at the head of all API. Same comment on silent-noisy now polluting thread-local storage. and again and again but THIS virDomainEvent*New was already reporting. Okay, you win - we SHOULD be reporting everywhere, even if we end up discarding the error object instead of sending it across rpc because we hit oom. A couple of suggestions, but no major errors, and I know you are planning on doing some of the double-oom cleanup in a later patch. ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/21/2013 03:14 AM, Michal Privoznik wrote: Ah, now that I look at the label, you're right! Yes, s/no_memory/error/ would avoid my confusion (generally, I've been assuming that a no_memory label implied an OOM report). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 23.05.2013 15:04, John Ferlan wrote: Right, it took me a while to parse the coverity output, and here's the part of the original code that confused me: if (!virHashLookup(table->hashTable, name)) { if (copyName) { name = strdup(name); Yup, that due to the bug in virNWFilterHashTablePut(). Michal

On 23.05.2013 15:56, John Ferlan wrote: Yup, that's exactly the patch that I wrote (haven't send it though yet). ACK if you want to push it. Michal

On 05/20/2013 11:55 AM, Michal Privoznik wrote: Unintentional change in return value from 0 to 1; I'd prefer keeping it at 0. Why snprintf only to strdup it? You can ditch dev_name, and just do: if (virAsprintf(&ret, "veth%d.%d", veid, ifNo) < 0) virReportOOMError(); return ret; ACK with those fixes. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Currently, @prompt member within _virConnectCredential struct is const char. This violates const correctness as we are not just strdup()-ing the value, we are even changing it (e.g. in virNetSSHKbIntCb()). --- include/libvirt/libvirt.h.in | 2 +- src/remote/remote_driver.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 1804c93..1bd3d1a 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -1294,7 +1294,7 @@ typedef enum { struct _virConnectCredential { int type; /* One of virConnectCredentialType constants */ - const char *prompt; /* Prompt to show to user */ + char *prompt; /* Prompt to show to user */ const char *challenge; /* Additional challenge to show */ const char *defresult; /* Optional default result */ char *result; /* Result to be filled with user response (or defresult) */ diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 13212d0..97be2a0 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -3733,7 +3733,7 @@ static int remoteAuthMakeCredentials(sasl_interact_t *interact, } if (interact[*ncred].challenge) (*cred)[*ncred].challenge = interact[ninteract].challenge; - (*cred)[*ncred].prompt = interact[ninteract].prompt; + (*cred)[*ncred].prompt = (char *) interact[ninteract].prompt; if (interact[*ncred].defresult) (*cred)[*ncred].defresult = interact[ninteract].defresult; (*cred)[*ncred].result = NULL; -- 1.8.2.1

--- src/qemu/qemu_capabilities.c | 79 +++----- src/qemu/qemu_cgroup.c | 4 +- src/qemu/qemu_command.c | 428 +++++++++++++++++-------------------------- src/qemu/qemu_conf.c | 64 +++---- src/qemu/qemu_domain.c | 26 +-- src/qemu/qemu_driver.c | 129 ++++--------- src/qemu/qemu_hotplug.c | 15 +- src/qemu/qemu_migration.c | 17 +- src/qemu/qemu_monitor_json.c | 63 ++----- src/qemu/qemu_monitor_text.c | 15 +- src/qemu/qemu_process.c | 64 +++---- 11 files changed, 333 insertions(+), 571 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 82129a2..f0471f4 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -369,8 +369,8 @@ virQEMUCapsParseMachineTypesStr(const char *output, if (!(t = strchr(p, ' ')) || (next && t >= next)) continue; - if (!(name = strndup(p, t - p))) - goto no_memory; + if (VIR_STRNDUP(name, p, t - p) < 0) + return -1; p = t; if ((t = strstr(p, "(default)")) && (!next || t < next)) @@ -381,9 +381,9 @@ virQEMUCapsParseMachineTypesStr(const char *output, if (!(t = strchr(p, ')')) || (next && t >= next)) continue; - if (!(canonical = strndup(p, t - p))) { + if (VIR_STRNDUP(canonical, p, t - p) < 0) { VIR_FREE(name); - goto no_memory; + return -1; } } @@ -391,7 +391,8 @@ virQEMUCapsParseMachineTypesStr(const char *output, VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1) < 0) { VIR_FREE(name); VIR_FREE(canonical); - goto no_memory; + virReportOOMError(); + return -1; } qemuCaps->nmachineTypes++; if (canonical) { @@ -408,10 +409,6 @@ virQEMUCapsParseMachineTypesStr(const char *output, virQEMUCapsSetDefaultMachine(qemuCaps, defIdx); return 0; - -no_memory: - virReportOOMError(); - return -1; } static int @@ -506,10 +503,8 @@ virQEMUCapsParseX86Models(const char *output, len -= 2; } - if (!(qemuCaps->cpuDefinitions[qemuCaps->ncpuDefinitions - 1] = strndup(p, len))) { - virReportOOMError(); + if (VIR_STRNDUP(qemuCaps->cpuDefinitions[qemuCaps->ncpuDefinitions - 1], p, len) < 0) goto cleanup; - } } while ((p = next)); ret = 0; @@ -559,10 +554,8 @@ virQEMUCapsParsePPCModels(const char *output, len = t - p - 1; - if (!(qemuCaps->cpuDefinitions[qemuCaps->ncpuDefinitions - 1] = strndup(p, len))) { - virReportOOMError(); + if (VIR_STRNDUP(qemuCaps->cpuDefinitions[qemuCaps->ncpuDefinitions - 1], p, len) < 0) goto cleanup; - } } while ((p = next)); ret = 0; @@ -1512,10 +1505,8 @@ virQEMUCapsParseDeviceStrObjectTypes(const char *str, virReportOOMError(); goto cleanup; } - if (!(typelist[ntypelist-1] = strndup(tmp, end-tmp))) { - virReportOOMError(); + if (VIR_STRNDUP(typelist[ntypelist - 1], tmp, end-tmp) < 0) goto cleanup; - } } *types = typelist; @@ -1569,10 +1560,8 @@ virQEMUCapsParseDeviceStrObjectProps(const char *str, virReportOOMError(); goto cleanup; } - if (!(proplist[nproplist-1] = strndup(tmp, end-tmp))) { - virReportOOMError(); + if (VIR_STRNDUP(proplist[nproplist - 1], tmp, end-tmp) < 0) goto cleanup; - } } *props = proplist; @@ -1741,8 +1730,8 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) goto no_memory; ret->ncpuDefinitions = qemuCaps->ncpuDefinitions; for (i = 0 ; i < qemuCaps->ncpuDefinitions ; i++) { - if (!(ret->cpuDefinitions[i] = strdup(qemuCaps->cpuDefinitions[i]))) - goto no_memory; + if (VIR_STRDUP(ret->cpuDefinitions[i], qemuCaps->cpuDefinitions[i]) < 0) + goto error; } if (VIR_ALLOC_N(ret->machineTypes, qemuCaps->nmachineTypes) < 0) @@ -1751,17 +1740,16 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) goto no_memory; ret->nmachineTypes = qemuCaps->nmachineTypes; for (i = 0 ; i < qemuCaps->nmachineTypes ; i++) { - if (!(ret->machineTypes[i] = strdup(qemuCaps->machineTypes[i]))) - goto no_memory; - if (qemuCaps->machineAliases[i] && - !(ret->machineAliases[i] = strdup(qemuCaps->machineAliases[i]))) - goto no_memory; + if (VIR_STRDUP(ret->machineTypes[i], qemuCaps->machineTypes[i]) < 0 || + VIR_STRDUP(ret->machineAliases[i], qemuCaps->machineAliases[i]) < 0) + goto error; } return ret; no_memory: virReportOOMError(); +error: virObjectUnref(ret); return NULL; } @@ -1863,11 +1851,10 @@ unsigned int virQEMUCapsGetKVMVersion(virQEMUCapsPtr qemuCaps) int virQEMUCapsAddCPUDefinition(virQEMUCapsPtr qemuCaps, const char *name) { - char *tmp = strdup(name); - if (!tmp) { - virReportOOMError(); + char *tmp; + + if (VIR_STRDUP(tmp, name) < 0) return -1; - } if (VIR_EXPAND_N(qemuCaps->cpuDefinitions, qemuCaps->ncpuDefinitions, 1) < 0) { VIR_FREE(tmp); virReportOOMError(); @@ -1912,12 +1899,11 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, if (VIR_ALLOC(mach) < 0) goto no_memory; if (qemuCaps->machineAliases[i]) { - if (!(mach->name = strdup(qemuCaps->machineAliases[i]))) - goto no_memory; - if (!(mach->canonical = strdup(qemuCaps->machineTypes[i]))) + if (VIR_STRDUP(mach->name, qemuCaps->machineAliases[i]) < 0 || + VIR_STRDUP(mach->canonical, qemuCaps->machineTypes[i]) < 0) goto no_memory; } else { - if (!(mach->name = strdup(qemuCaps->machineTypes[i]))) + if (VIR_STRDUP(mach->name, qemuCaps->machineTypes[i]) < 0) goto no_memory; } (*machines)[i] = mach; @@ -2106,16 +2092,9 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, } for (i = 0 ; i < nmachines ; i++) { - if (machines[i]->alias) { - if (!(qemuCaps->machineAliases[i] = strdup(machines[i]->alias))) { - virReportOOMError(); - goto cleanup; - } - } - if (!(qemuCaps->machineTypes[i] = strdup(machines[i]->name))) { - virReportOOMError(); + if (VIR_STRDUP(qemuCaps->machineAliases[i], machines[i]->alias) < 0 || + VIR_STRDUP(qemuCaps->machineTypes[i], machines[i]->name) < 0) goto cleanup; - } if (machines[i]->isDefault) defIdx = i; } @@ -2666,8 +2645,8 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, struct stat sb; int rv; - if (!(qemuCaps->binary = strdup(binary))) - goto no_memory; + if (VIR_STRDUP(qemuCaps->binary, binary) < 0) + goto error; /* We would also want to check faccessat if we cared about ACLs, * but we don't. */ @@ -2697,8 +2676,6 @@ virQEMUCapsPtr virQEMUCapsNewForBinary(const char *binary, return qemuCaps; -no_memory: - virReportOOMError(); error: virObjectUnref(qemuCaps); qemuCaps = NULL; @@ -2748,10 +2725,8 @@ virQEMUCapsCacheNew(const char *libDir, if (!(cache->binaries = virHashCreate(10, virQEMUCapsHashDataFree))) goto error; - if (!(cache->libDir = strdup(libDir))) { - virReportOOMError(); + if (VIR_STRDUP(cache->libDir, libDir) < 0) goto error; - } cache->runUid = runUid; cache->runGid = runGid; diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index ff9a075..6feeecd 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -32,6 +32,7 @@ #include "virerror.h" #include "domain_audit.h" #include "virscsi.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -695,8 +696,7 @@ int qemuInitCgroup(virQEMUDriverPtr driver, goto cleanup; } - if (!(res->partition = strdup("/machine"))) { - virReportOOMError(); + if (VIR_STRDUP(res->partition, "/machine") < 0) { VIR_FREE(res); goto cleanup; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 33a1910..22c4e4c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -325,10 +325,8 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, return -1; } else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { - if (!(brname = strdup(virDomainNetGetActualBridgeName(net)))) { - virReportOOMError(); + if (VIR_STRDUP(brname, virDomainNetGetActualBridgeName(net)) < 0) return -1; - } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Network type %d is not supported"), @@ -340,10 +338,8 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, STRPREFIX(net->ifname, VIR_NET_GENERATED_PREFIX) || strchr(net->ifname, '%')) { VIR_FREE(net->ifname); - if (!(net->ifname = strdup(VIR_NET_GENERATED_PREFIX "%d"))) { - virReportOOMError(); + if (VIR_STRDUP(net->ifname, VIR_NET_GENERATED_PREFIX "%d") < 0) goto cleanup; - } /* avoid exposing vnet%d in getXMLDesc or error outputs */ template_ifname = true; } @@ -541,17 +537,10 @@ static int qemuAssignDeviceDiskAliasLegacy(virDomainDiskDefPtr disk) { char *dev_name; - if (disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && - STREQ(disk->dst, "hdc")) - dev_name = strdup("cdrom"); - else - dev_name = strdup(disk->dst); - - if (!dev_name) { - virReportOOMError(); + if (VIR_STRDUP(dev_name, + disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM && + STREQ(disk->dst, "hdc") ? "cdrom" : disk->dst) < 0) return -1; - } - disk->info.alias = dev_name; return 0; } @@ -568,10 +557,7 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk, return NULL; } } else { - if (!(ret = strdup(disk->info.alias))) { - virReportOOMError(); - return NULL; - } + ignore_value(VIR_STRDUP(ret, disk->info.alias)); } return ret; } @@ -2489,13 +2475,11 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) if (port) { *port = '\0'; port += skip; - disk->hosts[disk->nhosts-1].port = strdup(port); - if (!disk->hosts[disk->nhosts-1].port) - goto no_memory; + if (VIR_STRDUP(disk->hosts[disk->nhosts - 1].port, port) < 0) + goto error; } else { - disk->hosts[disk->nhosts-1].port = strdup("6789"); - if (!disk->hosts[disk->nhosts-1].port) - goto no_memory; + if (VIR_STRDUP(disk->hosts[disk->nhosts - 1].port, "6789") < 0) + goto error; } parts = virStringSplit(hostport, "\\:", 0); @@ -2513,6 +2497,7 @@ static int qemuAddRBDHost(virDomainDiskDefPtr disk, char *hostport) no_memory: virReportOOMError(); +error: VIR_FREE(disk->hosts[disk->nhosts-1].port); VIR_FREE(disk->hosts[disk->nhosts-1].name); return -1; @@ -2526,9 +2511,8 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) p = strchr(disk->src, ':'); if (p) { - options = strdup(p + 1); - if (!options) - goto no_memory; + if (VIR_STRDUP(options, p + 1) < 0) + goto error; *p = '\0'; } @@ -2553,11 +2537,9 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) *e = '\0'; } - if (STRPREFIX(p, "id=")) { - disk->auth.username = strdup(p + strlen("id=")); - if (!disk->auth.username) - goto no_memory; - } + if (STRPREFIX(p, "id=") && + VIR_STRDUP(disk->auth.username, p + strlen("id=")) < 0) + goto error; if (STRPREFIX(p, "mon_host=")) { char *h, *sep; @@ -2584,9 +2566,8 @@ static int qemuParseRBDString(virDomainDiskDefPtr disk) VIR_FREE(options); return 0; -no_memory: +error: VIR_FREE(options); - virReportOOMError(); return -1; } @@ -2626,9 +2607,8 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, def->nhosts = 0; /* set to 1 once everything succeeds */ if (def->hosts->transport != VIR_DOMAIN_DISK_PROTO_TRANS_UNIX) { - def->hosts->name = strdup(uri->server); - if (!def->hosts->name) - goto no_memory; + if (VIR_STRDUP(def->hosts->name, uri->server) < 0) + goto error; if (virAsprintf(&def->hosts->port, "%d", uri->port) < 0) goto no_memory; @@ -2638,9 +2618,8 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, if (uri->query) { if (STRPREFIX(uri->query, "socket=")) { sock = strchr(uri->query, '=') + 1; - def->hosts->socket = strdup(sock); - if (!def->hosts->socket) - goto no_memory; + if (VIR_STRDUP(def->hosts->socket, sock) < 0) + goto error; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid query parameter '%s'"), uri->query); @@ -2651,9 +2630,8 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, if (uri->path) { volimg = uri->path + 1; /* skip the prefix slash */ VIR_FREE(def->src); - def->src = strdup(volimg); - if (!def->src) - goto no_memory; + if (VIR_STRDUP(def->src, volimg) < 0) + goto error; } else { VIR_FREE(def->src); def->src = NULL; @@ -2664,9 +2642,8 @@ qemuParseDriveURIString(virDomainDiskDefPtr def, virURIPtr uri, if (secret) *secret = '\0'; - def->auth.username = strdup(uri->user); - if (!def->auth.username) - goto no_memory; + if (VIR_STRDUP(def->auth.username, uri->user) < 0) + goto error; } def->nhosts = 1; @@ -2746,7 +2723,8 @@ qemuParseNBDString(virDomainDiskDefPtr disk) *src++ = '\0'; h->transport = VIR_DOMAIN_DISK_PROTO_TRANS_UNIX; - h->socket = strdup(host + strlen("unix:")); + if (VIR_STRDUP(h->socket, host + strlen("unix:")) < 0) + goto error; } else { port = strchr(host, ':'); if (!port) { @@ -2756,23 +2734,20 @@ qemuParseNBDString(virDomainDiskDefPtr disk) } *port++ = '\0'; - h->name = strdup(host); - if (!h->name) - goto no_memory; + if (VIR_STRDUP(h->name, host) < 0) + goto error; src = strchr(port, ':'); if (src) *src++ = '\0'; - h->port = strdup(port); - if (!h->port) - goto no_memory; + if (VIR_STRDUP(h->port, port) < 0) + goto error; } if (src && STRPREFIX(src, "exportname=")) { - src = strdup(strchr(src, '=') + 1); - if (!src) - goto no_memory; + if (VIR_STRDUP(src, strchr(src, '=') + 1) < 0) + goto error; } else { src = NULL; } @@ -2822,9 +2797,8 @@ qemuBuildDriveURIString(virConnectPtr conn, transp = virDomainDiskProtocolTransportTypeToString(disk->hosts->transport); if (disk->hosts->transport == VIR_DOMAIN_DISK_PROTO_TRANS_TCP) { - tmpscheme = strdup(scheme); - if (tmpscheme == NULL) - goto no_memory; + if (VIR_STRDUP(tmpscheme, scheme) < 0) + goto cleanup; } else { if (virAsprintf(&tmpscheme, "%s+%s", scheme, transp) < 0) goto no_memory; @@ -5643,9 +5617,10 @@ qemuBuildCpuArgStr(const virQEMUDriverPtr driver, } virBufferAddLit(&buf, "host"); } else { - if (VIR_ALLOC(guest) < 0 || - (cpu->vendor_id && !(guest->vendor_id = strdup(cpu->vendor_id)))) + if (VIR_ALLOC(guest) < 0) goto no_memory; + if (VIR_STRDUP(guest->vendor_id, cpu->vendor_id) < 0) + goto cleanup; guest->arch = host->arch; if (cpu->match == VIR_CPU_MATCH_MINIMUM) @@ -7282,8 +7257,8 @@ qemuBuildCommandLine(virConnectPtr conn, virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("network disks are only supported with -drive")); } else { - if (!(file = strdup(disk->src))) { - goto no_memory; + if (VIR_STRDUP(file, disk->src) < 0) { + goto error; } } @@ -8078,8 +8053,8 @@ qemuBuildCommandLine(virConnectPtr conn, goto error; } - if (!(optstr = strdup(model))) - goto no_memory; + if (VIR_STRDUP(optstr, model) < 0) + goto error; } virCommandAddArg(cmd, optstr); VIR_FREE(optstr); @@ -8549,17 +8524,16 @@ static int qemuStringToArgvEnv(const char *args, next = strchr(curr, '\n'); if (next) { - arg = strndup(curr, next-curr); + if (VIR_STRNDUP(arg, curr, next - curr) < 0) + goto error; if (*next == '\'' || *next == '"') next++; } else { - arg = strdup(curr); + if (VIR_STRDUP(arg, curr) < 0) + goto error; } - if (!arg) - goto no_memory; - if (argalloc == argcount) { if (VIR_REALLOC_N(arglist, argalloc+10) < 0) { VIR_FREE(arg); @@ -8609,13 +8583,14 @@ static int qemuStringToArgvEnv(const char *args, return 0; no_memory: + virReportOOMError(); +error: for (i = 0 ; progenv && progenv[i] ; i++) VIR_FREE(progenv[i]); VIR_FREE(progenv); for (i = 0 ; i < argcount ; i++) VIR_FREE(arglist[i]); VIR_FREE(arglist); - virReportOOMError(); return -1; } @@ -8689,14 +8664,14 @@ qemuParseKeywords(const char *str, separator = endmark; } - if (!(keyword = strndup(start, separator - start))) - goto no_memory; + if (VIR_STRNDUP(keyword, start, separator - start) < 0) + goto error; if (separator < endmark) { separator++; - if (!(value = strndup(separator, endmark - separator))) { + if (VIR_STRNDUP(value, separator, endmark - separator) < 0) { VIR_FREE(keyword); - goto no_memory; + goto error; } if (strchr(value, ',')) { char *p = strchr(value, ',') + 1; @@ -8800,11 +8775,8 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; def->protocol = VIR_DOMAIN_DISK_PROTOCOL_RBD; - def->src = strdup(p + strlen("rbd:")); - if (!def->src) { - virReportOOMError(); + if (VIR_STRDUP(def->src, p + strlen("rbd:")) < 0) goto error; - } /* old-style CEPH_ARGS env variable is parsed later */ if (!old_style_ceph_args && qemuParseRBDString(def) < 0) goto cleanup; @@ -8829,11 +8801,8 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, def->type = VIR_DOMAIN_DISK_TYPE_NETWORK; def->protocol = VIR_DOMAIN_DISK_PROTOCOL_SHEEPDOG; - def->src = strdup(p + strlen("sheepdog:")); - if (!def->src) { - virReportOOMError(); + if (VIR_STRDUP(def->src, p + strlen("sheepdog:")) < 0) goto error; - } /* def->src must be [vdiname] or [host]:[port]:[vdiname] */ port = strchr(def->src, ':'); @@ -8852,18 +8821,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } def->nhosts = 1; def->hosts->name = def->src; - def->hosts->port = strdup(port); - if (!def->hosts->port) { - virReportOOMError(); + if (VIR_STRDUP(def->hosts->port, port) < 0) goto error; - } def->hosts->transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; def->hosts->socket = NULL; - def->src = strdup(vdi); - if (!def->src) { - virReportOOMError(); + if (VIR_STRDUP(def->src, vdi) < 0) goto error; - } } VIR_FREE(p); @@ -8888,11 +8851,8 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(values[i], "floppy")) def->device = VIR_DOMAIN_DISK_DEVICE_FLOPPY; } else if (STREQ(keywords[i], "format")) { - def->driverName = strdup("qemu"); - if (!def->driverName) { - virReportOOMError(); + if (VIR_STRDUP(def->driverName, "qemu") < 0) goto error; - } def->format = virStorageFileFormatTypeFromString(values[i]); } else if (STREQ(keywords[i], "cache")) { if (STREQ(values[i], "off") || @@ -9032,21 +8992,19 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } if (def->bus == VIR_DOMAIN_DISK_BUS_IDE) { - def->dst = strdup("hda"); + ignore_value(VIR_STRDUP(def->dst, "hda")); } else if (def->bus == VIR_DOMAIN_DISK_BUS_SCSI) { - def->dst = strdup("sda"); + ignore_value(VIR_STRDUP(def->dst, "sda")); } else if (def->bus == VIR_DOMAIN_DISK_BUS_VIRTIO) { - def->dst = strdup("vda"); + ignore_value(VIR_STRDUP(def->dst, "vda")); } else if (def->bus == VIR_DOMAIN_DISK_BUS_XEN) { - def->dst = strdup("xvda"); + ignore_value(VIR_STRDUP(def->dst, "xvda")); } else { - def->dst = strdup("hda"); + ignore_value(VIR_STRDUP(def->dst, "hda")); } - if (!def->dst) { - virReportOOMError(); + if (!def->dst) goto error; - } if (STREQ(def->dst, "xvda")) def->dst[3] = 'a' + idx; else @@ -9398,14 +9356,12 @@ qemuParseCommandLineChr(virDomainChrSourceDefPtr source, source->type = VIR_DOMAIN_CHR_TYPE_PTY; } else if (STRPREFIX(val, "file:")) { source->type = VIR_DOMAIN_CHR_TYPE_FILE; - source->data.file.path = strdup(val+strlen("file:")); - if (!source->data.file.path) - goto no_memory; + if (VIR_STRDUP(source->data.file.path, val + strlen("file:")) < 0) + goto error; } else if (STRPREFIX(val, "pipe:")) { source->type = VIR_DOMAIN_CHR_TYPE_PIPE; - source->data.file.path = strdup(val+strlen("pipe:")); - if (!source->data.file.path) - goto no_memory; + if (VIR_STRDUP(source->data.file.path, val + strlen("pipe:")) < 0) + goto error; } else if (STREQ(val, "stdio")) { source->type = VIR_DOMAIN_CHR_TYPE_STDIO; } else if (STRPREFIX(val, "udp:")) { @@ -9416,40 +9372,32 @@ qemuParseCommandLineChr(virDomainChrSourceDefPtr source, host2 = svc1 ? strchr(svc1, '@') : NULL; svc2 = host2 ? strchr(host2, ':') : NULL; - if (svc1 && (svc1 != val)) { - source->data.udp.connectHost = strndup(val, svc1-val); - - if (!source->data.udp.connectHost) - goto no_memory; - } + if (svc1 && svc1 != val && + VIR_STRNDUP(source->data.udp.connectHost, val, svc1 - val) < 0) + goto error; if (svc1) { svc1++; - if (host2) - source->data.udp.connectService = strndup(svc1, host2-svc1); - else - source->data.udp.connectService = strdup(svc1); - if (!source->data.udp.connectService) - goto no_memory; + if ((host2 && VIR_STRNDUP(source->data.udp.connectService, + svc1, host2 - svc1) < 0) || + (!host2 && VIR_STRDUP(source->data.udp.connectService, + svc1) < 0)) + goto error; } if (host2) { host2++; - if (svc2 && (svc2 != host2)) { - source->data.udp.bindHost = strndup(host2, svc2-host2); - - if (!source->data.udp.bindHost) - goto no_memory; - } + if (svc2 && svc2 != host2 && + VIR_STRNDUP(source->data.udp.bindHost, host2, svc2 - host2) < 0) + goto no_memory; } if (svc2) { svc2++; if (STRNEQ(svc2, "0")) { - source->data.udp.bindService = strdup(svc2); - if (!source->data.udp.bindService) - goto no_memory; + if (VIR_STRDUP(source->data.udp.bindService, svc2) < 0) + goto error; } } } else if (STRPREFIX(val, "tcp:") || @@ -9472,37 +9420,25 @@ qemuParseCommandLineChr(virDomainChrSourceDefPtr source, if (opt && strstr(opt, "server")) source->data.tcp.listen = true; - source->data.tcp.host = strndup(val, svc-val); - if (!source->data.tcp.host) - goto no_memory; + if (VIR_STRNDUP(source->data.tcp.host, val, svc - val) < 0) + goto error; svc++; - if (opt) { - source->data.tcp.service = strndup(svc, opt-svc); - } else { - source->data.tcp.service = strdup(svc); - } - if (!source->data.tcp.service) - goto no_memory; + if ((opt && VIR_STRNDUP(source->data.tcp.service, svc, opt - svc) < 0) || + (!opt && VIR_STRDUP(source->data.tcp.service, svc) < 0)) + goto error; } else if (STRPREFIX(val, "unix:")) { const char *opt; val += strlen("unix:"); opt = strchr(val, ','); source->type = VIR_DOMAIN_CHR_TYPE_UNIX; - if (opt) { - if (strstr(opt, "listen")) - source->data.nix.listen = true; - source->data.nix.path = strndup(val, opt-val); - } else { - source->data.nix.path = strdup(val); - } - if (!source->data.nix.path) - goto no_memory; + if ((opt && VIR_STRNDUP(source->data.nix.path, val, opt - val) < 0) || + (!opt && VIR_STRDUP(source->data.nix.path, val) < 0)) + goto error; } else if (STRPREFIX(val, "/dev")) { source->type = VIR_DOMAIN_CHR_TYPE_DEV; - source->data.file.path = strdup(val); - if (!source->data.file.path) - goto no_memory; + if (VIR_STRDUP(source->data.file.path, val) < 0) + goto error; } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("unknown character device syntax %s"), val); @@ -9555,13 +9491,9 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, next++; if (p == val) { - if (next) - model = strndup(p, next - p - 1); - else - model = strdup(p); - - if (!model) - goto no_memory; + if ((next && VIR_STRNDUP(model, p, next - p - 1) < 0) || + (!next && VIR_STRDUP(model, p) < 0)) + goto error; if (!STREQ(model, "qemu32") && !STREQ(model, "qemu64")) { if (!(cpu = qemuInitGuestCPU(dom))) @@ -9584,13 +9516,9 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (*p == '\0' || *p == ',') goto syntax; - if (next) - feature = strndup(p, next - p - 1); - else - feature = strdup(p); - - if (!feature) - goto no_memory; + if ((next && VIR_STRNDUP(feature, p, next - p - 1) < 0) || + (!next && VIR_STRDUP(feature, p) < 0)) + goto error; if (STREQ(feature, "kvmclock")) { bool present = (policy == VIR_CPU_FEATURE_REQUIRE); @@ -9648,13 +9576,9 @@ qemuParseCommandLineCPU(virDomainDefPtr dom, if (*p == '\0' || *p == ',') goto syntax; - if (next) - feature = strndup(p, next - p - 1); - else - feature = strdup(p); - - if (!feature) - goto no_memory; + if ((next && VIR_STRNDUP(feature, p, next - p - 1) < 0) || + (!next && VIR_STRDUP(feature, p) < 0)) + goto error; dom->features |= (1 << VIR_DOMAIN_FEATURE_HYPERV); @@ -9872,8 +9796,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, def->onCrash = VIR_DOMAIN_LIFECYCLE_DESTROY; def->onPoweroff = VIR_DOMAIN_LIFECYCLE_DESTROY; def->virtType = VIR_DOMAIN_VIRT_QEMU; - if (!(def->emulator = strdup(progargv[0]))) - goto no_memory; + if (VIR_STRDUP(def->emulator, progargv[0]) < 0) + goto error; if (strstr(def->emulator, "kvm")) { def->virtType = VIR_DOMAIN_VIRT_KVM; @@ -9883,12 +9807,12 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, if (strstr(def->emulator, "xenner")) { def->virtType = VIR_DOMAIN_VIRT_KVM; - def->os.type = strdup("xen"); + if (VIR_STRDUP(def->os.type, "xen") < 0) + goto error; } else { - def->os.type = strdup("hvm"); + if (VIR_STRDUP(def->os.type, "hvm") < 0) + goto error; } - if (!def->os.type) - goto no_memory; if (STRPREFIX(def->emulator, "qemu")) path = def->emulator; @@ -9951,10 +9875,9 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, if (STRPREFIX(val, "unix:")) { /* -vnc unix:/some/big/path */ - vnc->data.vnc.socket = strdup(val + 5); - if (!vnc->data.vnc.socket) { + if (VIR_STRDUP(vnc->data.vnc.socket, val + 5) < 0) { virDomainGraphicsDefFree(vnc); - goto no_memory; + goto error; } } else { /* @@ -9994,10 +9917,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, } if (*opts == ',') { - char *orig_opts = strdup(opts + 1); - if (!orig_opts) { + char *orig_opts; + + if (VIR_STRDUP(orig_opts, opts + 1) < 0) { virDomainGraphicsDefFree(vnc); - goto no_memory; + goto error; } opts = orig_opts; @@ -10093,9 +10017,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, disk->type = VIR_DOMAIN_DISK_TYPE_FILE; if (STREQ(arg, "-cdrom")) { disk->device = VIR_DOMAIN_DISK_DEVICE_CDROM; - disk->dst = strdup("hdc"); - if (!disk->dst) - goto no_memory; + if (VIR_STRDUP(disk->dst, "hdc") < 0) + goto error; disk->readonly = true; } else { if (STRPREFIX(arg, "-fd")) { @@ -10108,13 +10031,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, else disk->bus = VIR_DOMAIN_DISK_BUS_SCSI; } - disk->dst = strdup(arg + 1); - if (!disk->dst) - goto no_memory; + if (VIR_STRDUP(disk->dst, arg + 1) < 0) + goto error; } - disk->src = strdup(val); - if (!disk->src) - goto no_memory; + if (VIR_STRDUP(disk->src, val) < 0) + goto error; if (disk->type == VIR_DOMAIN_DISK_TYPE_NETWORK) { char *port; @@ -10147,12 +10068,10 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto no_memory; disk->nhosts = 1; disk->hosts->name = disk->src; - disk->hosts->port = strdup(port); - if (!disk->hosts->port) - goto no_memory; - disk->src = strdup(vdi); - if (!disk->src) - goto no_memory; + if (VIR_STRDUP(disk->hosts->port, port) < 0) + goto error; + if (VIR_STRDUP(disk->src, vdi) < 0) + goto error; } break; case VIR_DOMAIN_DISK_PROTOCOL_GLUSTER: @@ -10199,24 +10118,24 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME; } else if (STREQ(arg, "-kernel")) { WANT_VALUE(); - if (!(def->os.kernel = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->os.kernel, val) < 0) + goto error; } else if (STREQ(arg, "-bios")) { WANT_VALUE(); - if (!(def->os.loader = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->os.loader, val) < 0) + goto error; } else if (STREQ(arg, "-initrd")) { WANT_VALUE(); - if (!(def->os.initrd = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->os.initrd, val) < 0) + goto error; } else if (STREQ(arg, "-append")) { WANT_VALUE(); - if (!(def->os.cmdline = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->os.cmdline, val) < 0) + goto error; } else if (STREQ(arg, "-dtb")) { WANT_VALUE(); - if (!(def->os.dtb = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->os.dtb, val) < 0) + goto error; } else if (STREQ(arg, "-boot")) { const char *token = NULL; WANT_VALUE(); @@ -10260,11 +10179,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, WANT_VALUE(); process = strstr(val, ",process="); if (process == NULL) { - if (!(def->name = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->name, val) < 0) + goto error; } else { - if (!(def->name = strndup(val, process - val))) - goto no_memory; + if (VIR_STRNDUP(def->name, val, process - val) < 0) + goto error; } if (STREQ(def->name, "")) VIR_FREE(def->name); @@ -10274,11 +10193,11 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, WANT_VALUE(); params = strchr(val, ','); if (params == NULL) { - if (!(def->os.machine = strdup(val))) - goto no_memory; + if (VIR_STRDUP(def->os.machine, val) < 0) + goto error; } else { - if (!(def->os.machine = strndup(val, params - val))) - goto no_memory; + if (VIR_STRNDUP(def->os.machine, val, params - val) < 0) + goto error; while (params++) { /* prepared for more "-machine" parameters */ @@ -10287,11 +10206,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, if (STRPREFIX(tmp, "dump-guest-core=")) { tmp += strlen("dump-guest-core="); - if (params) { - tmp = strndup(tmp, params - tmp); - if (tmp == NULL) - goto no_memory; - } + if (params && VIR_STRNDUP(tmp, tmp, params - tmp) < 0) + goto error; def->mem.dump_core = virDomainMemDumpTypeFromString(tmp); if (def->mem.dump_core <= 0) def->mem.dump_core = VIR_DOMAIN_MEM_DUMP_DEFAULT; @@ -10362,17 +10278,17 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, } else if (STRPREFIX(val, "disk:")) { if (VIR_ALLOC(disk) < 0) goto no_memory; - disk->src = strdup(val + strlen("disk:")); - if (!disk->src) - goto no_memory; + if (VIR_STRDUP(disk->src, val + strlen("disk:")) < 0) + goto error; if (STRPREFIX(disk->src, "/dev/")) disk->type = VIR_DOMAIN_DISK_TYPE_BLOCK; else disk->type = VIR_DOMAIN_DISK_TYPE_FILE; disk->device = VIR_DOMAIN_DISK_DEVICE_DISK; disk->bus = VIR_DOMAIN_DISK_BUS_USB; - if (!(disk->dst = strdup("sda")) || - VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) + if (VIR_STRDUP(disk->dst, "sda") < 0) + goto error; + if (VIR_REALLOC_N(def->disks, def->ndisks+1) < 0) goto no_memory; def->disks[def->ndisks++] = disk; disk = NULL; @@ -10474,9 +10390,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, def->watchdog->action = action; } else if (STREQ(arg, "-bootloader")) { WANT_VALUE(); - def->os.bootloader = strdup(val); - if (!def->os.bootloader) - goto no_memory; + if (VIR_STRDUP(def->os.bootloader, val) < 0) + goto error; } else if (STREQ(arg, "-vmwarevga")) { video = VIR_DOMAIN_VIDEO_TYPE_VMVGA; } else if (STREQ(arg, "-std-vga")) { @@ -10509,8 +10424,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, } else if (STREQ(arg, "-pidfile")) { WANT_VALUE(); if (pidfile) - if (!(*pidfile = strdup(val))) - goto no_memory; + if (VIR_STRDUP(*pidfile, val) < 0) + goto error; } else if (STREQ(arg, "-incoming")) { WANT_VALUE(); /* ignore, used via restore/migrate APIs */ @@ -10592,9 +10507,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, arg); if (VIR_REALLOC_N(cmd->args, cmd->num_args+1) < 0) goto no_memory; - cmd->args[cmd->num_args] = strdup(arg); - if (cmd->args[cmd->num_args] == NULL) - goto no_memory; + if (VIR_STRDUP(cmd->args[cmd->num_args], arg) < 0) + goto error; cmd->num_args++; } } @@ -10623,9 +10537,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, _("could not parse CEPH_ARGS '%s'"), ceph_args); goto error; } - hosts = strdup(strchr(ceph_args, ' ') + 1); - if (!hosts) - goto no_memory; + if (VIR_STRDUP(hosts, strchr(ceph_args, ' ') + 1) < 0) + goto error; first_rbd_disk->nhosts = 0; token = strtok_r(hosts, ",", &saveptr); while (token != NULL) { @@ -10636,17 +10549,16 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, port = strchr(token, ':'); if (port) { *port++ = '\0'; - port = strdup(port); - if (!port) { + if (VIR_STRDUP(port, port) < 0) { VIR_FREE(hosts); - goto no_memory; + goto error; } } first_rbd_disk->hosts[first_rbd_disk->nhosts].port = port; - first_rbd_disk->hosts[first_rbd_disk->nhosts].name = strdup(token); - if (!first_rbd_disk->hosts[first_rbd_disk->nhosts].name) { + if (VIR_STRDUP(first_rbd_disk->hosts[first_rbd_disk->nhosts].name, + token) < 0) { VIR_FREE(hosts); - goto no_memory; + goto error; } first_rbd_disk->hosts[first_rbd_disk->nhosts].transport = VIR_DOMAIN_DISK_PROTO_TRANS_TCP; first_rbd_disk->hosts[first_rbd_disk->nhosts].socket = NULL; @@ -10670,8 +10582,8 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, def->os.arch, virDomainVirtTypeToString(def->virtType)); if (defaultMachine != NULL) - if (!(def->os.machine = strdup(defaultMachine))) - goto no_memory; + if (VIR_STRDUP(def->os.machine, defaultMachine) < 0) + goto error; } if (!nographics && def->ngraphics == 0) { @@ -10682,15 +10594,13 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr qemuCaps, goto no_memory; sdl->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL; sdl->data.sdl.fullscreen = fullscreen; - if (display && - !(sdl->data.sdl.display = strdup(display))) { + if (VIR_STRDUP(sdl->data.sdl.display, display) < 0) { VIR_FREE(sdl); - goto no_memory; + goto error; } - if (xauth && - !(sdl->data.sdl.xauth = strdup(xauth))) { + if (VIR_STRDUP(sdl->data.sdl.xauth, xauth) < 0) { VIR_FREE(sdl); - goto no_memory; + goto error; } if (VIR_REALLOC_N(def->graphics, def->ngraphics+1) < 0) { @@ -10807,7 +10717,7 @@ static int qemuParseProcFileStrings(int pid_value, ssize_t len; char *tmp; size_t nstr = 0; - const char **str = NULL; + char **str = NULL; int i; if (virAsprintf(&path, "/proc/%d/%s", pid_value, name) < 0) { @@ -10825,10 +10735,8 @@ static int qemuParseProcFileStrings(int pid_value, goto cleanup; } - if (!(str[nstr-1] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(str[nstr-1], tmp) < 0) goto cleanup; - } /* Skip arg */ tmp += strlen(tmp); /* Skip \0 separator */ @@ -10843,7 +10751,7 @@ static int qemuParseProcFileStrings(int pid_value, str[nstr-1] = NULL; ret = nstr-1; - *list = str; + *list = (const char **) str; cleanup: if (ret < 0) { diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 41fe0b9..60c2370 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -140,8 +140,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) "%s/log/libvirt/qemu", LOCALSTATEDIR) < 0) goto no_memory; - if ((cfg->configBaseDir = strdup(SYSCONFDIR "/libvirt")) == NULL) - goto no_memory; + if (VIR_STRDUP(cfg->configBaseDir, SYSCONFDIR "/libvirt") < 0) + goto error; if (virAsprintf(&cfg->stateDir, "%s/run/libvirt/qemu", LOCALSTATEDIR) < 0) @@ -210,19 +210,17 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto no_memory; - if (!(cfg->vncListen = strdup("127.0.0.1"))) - goto no_memory; + if (VIR_STRDUP(cfg->vncListen, "127.0.0.1") < 0) + goto error; - if (!(cfg->vncTLSx509certdir - = strdup(SYSCONFDIR "/pki/libvirt-vnc"))) - goto no_memory; + if (VIR_STRDUP(cfg->vncTLSx509certdir, SYSCONFDIR "/pki/libvirt-vnc") < 0) + goto error; - if (!(cfg->spiceListen = strdup("127.0.0.1"))) - goto no_memory; + if (VIR_STRDUP(cfg->spiceListen, "127.0.0.1") < 0) + goto error; - if (!(cfg->spiceTLSx509certdir - = strdup(SYSCONFDIR "/pki/libvirt-spice"))) - goto no_memory; + if (VIR_STRDUP(cfg->spiceTLSx509certdir , SYSCONFDIR "/pki/libvirt-spice") < 0) + goto error; cfg->remotePortMin = QEMU_REMOTE_PORT_MIN; cfg->remotePortMax = QEMU_REMOTE_PORT_MAX; @@ -243,8 +241,8 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) } } #endif - if (!(cfg->bridgeHelperName = strdup("/usr/libexec/qemu-bridge-helper"))) - goto no_memory; + if (VIR_STRDUP(cfg->bridgeHelperName, "/usr/libexec/qemu-bridge-helper") < 0) + goto error; cfg->clearEmulatorCapabilities = true; @@ -350,8 +348,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, CHECK_TYPE(NAME, VIR_CONF_STRING); \ if (p && p->str) { \ VIR_FREE(VAR); \ - if (!(VAR = strdup(p->str))) \ - goto no_memory; \ + if (VIR_STRDUP(VAR, p->str) < 0) \ + goto cleanup; \ } GET_VALUE_BOOL("vnc_auto_unix_socket", cfg->vncAutoUnixSocket); @@ -382,16 +380,17 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, goto no_memory; for (i = 0, pp = p->list; pp; i++, pp = pp->next) { - if (!(cfg->securityDriverNames[i] = strdup(pp->str))) - goto no_memory; + if (VIR_STRDUP(cfg->securityDriverNames[i], pp->str) < 0) + goto cleanup; } cfg->securityDriverNames[len] = NULL; } else { CHECK_TYPE("security_driver", VIR_CONF_STRING); if (p && p->str) { - if (VIR_ALLOC_N(cfg->securityDriverNames, 2) < 0 || - !(cfg->securityDriverNames[0] = strdup(p->str))) + if (VIR_ALLOC_N(cfg->securityDriverNames, 2) < 0) goto no_memory; + if (VIR_STRDUP(cfg->securityDriverNames[0], p->str) < 0) + goto cleanup; cfg->securityDriverNames[1] = NULL; } @@ -518,8 +517,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, "list of strings")); goto cleanup; } - if (!(cfg->cgroupDeviceACL[i] = strdup(pp->str))) - goto no_memory; + if (VIR_STRDUP(cfg->cgroupDeviceACL[i], pp->str) < 0) + goto cleanup; } cfg->cgroupDeviceACL[i] = NULL; } @@ -618,10 +617,9 @@ virCapsPtr virQEMUDriverCreateCapabilities(virQEMUDriverPtr driver) for (i = 0; sec_managers[i]; i++) { doi = virSecurityManagerGetDOI(sec_managers[i]); model = virSecurityManagerGetModel(sec_managers[i]); - if (!(caps->host.secModels[i].model = strdup(model))) - goto no_memory; - if (!(caps->host.secModels[i].doi = strdup(doi))) - goto no_memory; + if (VIR_STRDUP(caps->host.secModels[i].model, model) < 0 || + VIR_STRDUP(caps->host.secModels[i].doi, doi) < 0) + goto error; VIR_DEBUG("Initialized caps for security driver \"%s\" with " "DOI \"%s\"", model, doi); } @@ -1140,10 +1138,8 @@ qemuSharedDeviceEntryCopy(const qemuSharedDeviceEntryPtr entry) } for (i = 0; i < entry->ref; i++) { - if (!(ret->domains[i] = strdup(entry->domains[i]))) { - virReportOOMError(); + if (VIR_STRDUP(ret->domains[i], entry->domains[i]) < 0) goto cleanup; - } ret->ref++; } @@ -1237,8 +1233,8 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, if (!(new_entry = qemuSharedDeviceEntryCopy(entry))) goto cleanup; - if ((VIR_EXPAND_N(new_entry->domains, new_entry->ref, 1) < 0) || - !(new_entry->domains[new_entry->ref - 1] = strdup(name))) { + if (VIR_EXPAND_N(new_entry->domains, new_entry->ref, 1) < 0 || + VIR_STRDUP(new_entry->domains[new_entry->ref - 1], name) < 0) { qemuSharedDeviceEntryFree(new_entry, NULL); virReportOOMError(); goto cleanup; @@ -1249,9 +1245,9 @@ qemuAddSharedDevice(virQEMUDriverPtr driver, goto cleanup; } } else { - if ((VIR_ALLOC(entry) < 0) || - (VIR_ALLOC_N(entry->domains, 1) < 0) || - !(entry->domains[0] = strdup(name))) { + if (VIR_ALLOC(entry) < 0 || + VIR_ALLOC_N(entry->domains, 1) < 0 || + VIR_STRDUP(entry->domains[0], name) < 0) { qemuSharedDeviceEntryFree(entry, NULL); virReportOOMError(); goto cleanup; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d7c2975..8a84cf0 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -727,14 +727,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV && !dev->data.net->model) { - if (def->os.arch == VIR_ARCH_S390 || - def->os.arch == VIR_ARCH_S390X) - dev->data.net->model = strdup("virtio"); - else - dev->data.net->model = strdup("rtl8139"); - - if (!dev->data.net->model) - goto no_memory; + if (VIR_STRDUP(dev->data.net->model, + def->os.arch == VIR_ARCH_S390 || + def->os.arch == VIR_ARCH_S390X ? "virtio" : "rtl8139") < 0) + goto cleanup; } /* set default disk types and drivers */ @@ -758,8 +754,8 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, } else { /* default driver if probing is forbidden */ if (!disk->driverName && - !(disk->driverName = strdup("qemu"))) - goto no_memory; + VIR_STRDUP(disk->driverName, "qemu") < 0) + goto cleanup; /* default disk format for drives */ if (disk->format == VIR_STORAGE_FILE_NONE && @@ -801,8 +797,10 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, if (virAsprintf(&dev->data.chr->source.data.nix.path, "%s/channel/target/%s.%s", cfg->libDir, def->name, - dev->data.chr->target.name) < 0) - goto no_memory; + dev->data.chr->target.name) < 0) { + virReportOOMError(); + goto cleanup; + } dev->data.chr->source.data.nix.listen = true; } @@ -811,10 +809,6 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, cleanup: virObjectUnref(cfg); return ret; - -no_memory: - virReportOOMError(); - goto cleanup; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 0665131..4dfac60 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1456,10 +1456,8 @@ qemuCanonicalizeMachine(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) if (STRNEQ(canon, def->os.machine)) { char *tmp; - if (!(tmp = strdup(canon))) { - virReportOOMError(); + if (VIR_STRDUP(tmp, canon) < 0) return -1; - } VIR_FREE(def->os.machine); def->os.machine = tmp; } @@ -2047,8 +2045,7 @@ static char *qemuDomainGetOSType(virDomainPtr dom) { if (!(vm = qemuDomObjFromDomain(dom))) goto cleanup; - if (!(type = strdup(vm->def->os.type))) - virReportOOMError(); + ignore_value(VIR_STRDUP(type, vm->def->os.type)); cleanup: if (vm) @@ -3426,7 +3423,7 @@ qemuDomainScreenshot(virDomainPtr dom, goto endjob; } - ret = strdup("image/x-portable-pixmap"); + ignore_value(VIR_STRDUP(ret, "image/x-portable-pixmap")); endjob: VIR_FORCE_CLOSE(tmp_fd); @@ -5174,11 +5171,8 @@ static char *qemuConnectDomainXMLFromNative(virConnectPtr conn, if (!def) goto cleanup; - if (!def->name && - !(def->name = strdup("unnamed"))) { - virReportOOMError(); + if (!def->name && VIR_STRDUP(def->name, "unnamed") < 0) goto cleanup; - } xml = qemuDomainDefFormatXML(driver, def, VIR_DOMAIN_XML_INACTIVE); @@ -5244,11 +5238,9 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, if ((actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) && (brname = virDomainNetGetActualBridgeName(net))) { - char *brnamecopy = strdup(brname); - if (!brnamecopy) { - virReportOOMError(); + char *brnamecopy; + if (VIR_STRDUP(brnamecopy, brname) < 0) goto cleanup; - } virDomainActualNetDefFree(net->data.network.actual); @@ -6923,9 +6915,7 @@ static char *qemuDomainGetSchedulerType(virDomainPtr dom, *nparams = 5; } - ret = strdup("posix"); - if (!ret) - virReportOOMError(); + ignore_value(VIR_STRDUP(ret, "posix")); cleanup: if (vm) @@ -6983,11 +6973,8 @@ qemuDomainParseDeviceWeightStr(char *deviceWeightStr, if (!p) goto error; - result[i].path = strndup(temp, p - temp); - if (!result[i].path) { - virReportOOMError(); + if (VIR_STRNDUP(result[i].path, temp, p - temp) < 0) goto cleanup; - } /* weight */ temp = p + 1; @@ -7351,13 +7338,8 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, } param->value.s = virBufferContentAndReset(&buf); } - if (!param->value.s) { - param->value.s = strdup(""); - if (!param->value.s) { - virReportOOMError(); - goto cleanup; - } - } + if (!param->value.s && VIR_STRDUP(param->value.s, "") < 0) + goto cleanup; param->type = VIR_TYPED_PARAM_STRING; if (virStrcpyStatic(param->field, VIR_DOMAIN_BLKIO_DEVICE_WEIGHT) == NULL) { @@ -7882,8 +7864,8 @@ qemuDomainGetNumaParameters(virDomainPtr dom, case 1: /* fill numa nodeset here */ if (flags & VIR_DOMAIN_AFFECT_CONFIG) { nodeset = virBitmapFormat(persistentDef->numatune.memory.nodemask); - if (!nodeset) - nodeset = strdup(""); + if (!nodeset && VIR_STRDUP(nodeset, "") < 0) + goto cleanup; } else { rc = virCgroupGetCpusetMems(priv->cgroup, &nodeset); if (rc != 0) { @@ -9560,10 +9542,8 @@ qemuDomainMigratePrepareTunnel(virConnectPtr dconn, if (dname) { VIR_FREE(def->name); - if (!(def->name = strdup(dname))) { - virReportOOMError(); + if (VIR_STRDUP(def->name, dname) < 0) goto cleanup; - } } ret = qemuMigrationPrepareTunnel(driver, dconn, @@ -9633,10 +9613,8 @@ qemuDomainMigratePrepare2(virConnectPtr dconn, if (dname) { VIR_FREE(def->name); - if (!(def->name = strdup(dname))) { - virReportOOMError(); + if (VIR_STRDUP(def->name, dname) < 0) goto cleanup; - } } /* Do not use cookies in v2 protocol, since the cookie @@ -9874,10 +9852,8 @@ qemuDomainMigratePrepare3(virConnectPtr dconn, if (dname) { VIR_FREE(def->name); - if (!(def->name = strdup(dname))) { - virReportOOMError(); + if (VIR_STRDUP(def->name, dname) < 0) goto cleanup; - } } ret = qemuMigrationPrepareDirect(driver, dconn, @@ -9939,10 +9915,8 @@ qemuDomainMigratePrepareTunnel3(virConnectPtr dconn, if (dname) { VIR_FREE(def->name); - if (!(def->name = strdup(dname))) { - virReportOOMError(); + if (VIR_STRDUP(def->name, dname) < 0) goto cleanup; - } } ret = qemuMigrationPrepareTunnel(driver, dconn, @@ -10959,9 +10933,8 @@ qemuDomainSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, if (snapdisk->snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL) { VIR_FREE(defdisk->src); - if (!(defdisk->src = strdup(snapdisk->file))) { + if (VIR_STRDUP(defdisk->src, snapdisk->file) < 0) { /* we cannot rollback here in a sane way */ - virReportOOMError(); goto cleanup; } defdisk->format = snapdisk->format; @@ -11250,9 +11223,8 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, } if (virAsprintf(&device, "drive-%s", disk->info.alias) < 0 || - !(source = strdup(snap->file)) || - (persistDisk && - !(persistSource = strdup(source)))) { + VIR_STRDUP(source, snap->file) < 0 || + (persistDisk && VIR_STRDUP(persistSource, source) < 0)) { virReportOOMError(); goto cleanup; } @@ -11329,12 +11301,9 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr driver, char *persistSource = NULL; struct stat st; - if (!(source = strdup(origdisk->src)) || - (persistDisk && - !(persistSource = strdup(source)))) { - virReportOOMError(); + if (VIR_STRDUP(source, origdisk->src) < 0 || + (persistDisk && VIR_STRDUP(persistSource, source) < 0)) goto cleanup; - } qemuDomainPrepareDiskChainElement(driver, vm, disk, origdisk->src, VIR_DISK_CHAIN_NO_ACCESS); @@ -11938,13 +11907,9 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain, if (update_current) snap->def->current = true; if (vm->current_snapshot) { - if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE)) { - snap->def->parent = strdup(vm->current_snapshot->def->name); - if (snap->def->parent == NULL) { - virReportOOMError(); + if (!(flags & VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE) && + VIR_STRDUP(snap->def->parent, vm->current_snapshot->def->name) < 0) goto cleanup; - } - } if (update_current) { vm->current_snapshot->def->current = false; if (qemuDomainSnapshotWriteMetadata(vm, vm->current_snapshot, @@ -12723,14 +12688,10 @@ qemuDomainSnapshotReparentChildren(void *payload, VIR_FREE(snap->def->parent); snap->parent = rep->parent; - if (rep->parent->def) { - snap->def->parent = strdup(rep->parent->def->name); - - if (snap->def->parent == NULL) { - virReportOOMError(); - rep->err = -1; - return; - } + if (rep->parent->def && + VIR_STRDUP(snap->def->parent, rep->parent->def->name) < 0) { + rep->err = -1; + return; } if (!snap->sibling) @@ -13637,10 +13598,8 @@ qemuDomainBlockCopy(virDomainPtr dom, const char *path, } if (!format && disk->mirrorFormat > 0) format = virStorageFileFormatTypeToString(disk->mirrorFormat); - if (!(mirror = strdup(dest))) { - virReportOOMError(); + if (VIR_STRDUP(mirror, dest) < 0) goto endjob; - } if (qemuDomainPrepareDiskChainElement(driver, vm, disk, dest, VIR_DISK_CHAIN_READ_WRITE) < 0) { @@ -14271,10 +14230,8 @@ qemuDomainGetDiskErrors(virDomainPtr dom, if (n == nerrors) break; - if (!(errors[n].disk = strdup(disk->dst))) { - virReportOOMError(); + if (VIR_STRDUP(errors[n].disk, disk->dst) < 0) goto endjob; - } errors[n].error = info->io_status; n++; } @@ -14331,15 +14288,13 @@ qemuDomainSetMetadata(virDomainPtr dom, switch ((virDomainMetadataType) type) { case VIR_DOMAIN_METADATA_DESCRIPTION: VIR_FREE(vm->def->description); - if (metadata && - !(vm->def->description = strdup(metadata))) - goto no_memory; + if (VIR_STRDUP(vm->def->description, metadata) < 0) + goto cleanup; break; case VIR_DOMAIN_METADATA_TITLE: VIR_FREE(vm->def->title); - if (metadata && - !(vm->def->title = strdup(metadata))) - goto no_memory; + if (VIR_STRDUP(vm->def->title, metadata) < 0) + goto cleanup; break; case VIR_DOMAIN_METADATA_ELEMENT: virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -14359,15 +14314,13 @@ qemuDomainSetMetadata(virDomainPtr dom, switch ((virDomainMetadataType) type) { case VIR_DOMAIN_METADATA_DESCRIPTION: VIR_FREE(persistentDef->description); - if (metadata && - !(persistentDef->description = strdup(metadata))) - goto no_memory; + if (VIR_STRDUP(persistentDef->description, metadata) < 0) + goto cleanup; break; case VIR_DOMAIN_METADATA_TITLE: VIR_FREE(persistentDef->title); - if (metadata && - !(persistentDef->title = strdup(metadata))) - goto no_memory; + if (VIR_STRDUP(persistentDef->title, metadata) < 0) + goto cleanup; break; case VIR_DOMAIN_METADATA_ELEMENT: virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", @@ -14393,9 +14346,6 @@ cleanup: virObjectUnref(caps); virObjectUnref(cfg); return ret; -no_memory: - virReportOOMError(); - goto cleanup; } static char * @@ -14453,10 +14403,7 @@ qemuDomainGetMetadata(virDomainPtr dom, goto cleanup; } - if (!(ret = strdup(field))) { - virReportOOMError(); - goto cleanup; - } + ignore_value(VIR_STRDUP(ret, field)); cleanup: if (vm) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 77d9f4f..c0aa824 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1353,10 +1353,8 @@ qemuDomainNetGetBridgeName(virConnectPtr conn, virDomainNetDefPtr net) goto cleanup; } /* we need a copy, not just a pointer to the original */ - if (!(brname = strdup(tmpbr))) { - virReportOOMError(); + if (VIR_STRDUP(brname, tmpbr) < 0) goto cleanup; - } } else if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK) { int active; virErrorPtr errobj; @@ -1609,11 +1607,8 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, } /* ifname: check if it's set in newdev. If not, retain the autogenerated one */ - if (!(newdev->ifname || - (newdev->ifname = strdup(olddev->ifname)))) { - virReportOOMError(); + if (!newdev->ifname && VIR_STRDUP(newdev->ifname, olddev->ifname) < 0) goto cleanup; - } if (STRNEQ_NULLABLE(olddev->ifname, newdev->ifname)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device tap name")); @@ -1638,11 +1633,9 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, goto cleanup; } /* grab alias from olddev if not set in newdev */ - if (!(newdev->info.alias || - (newdev->info.alias = strdup(olddev->info.alias)))) { - virReportOOMError(); + if (!newdev->info.alias && + VIR_STRDUP(newdev->info.alias, olddev->info.alias) < 0) goto cleanup; - } if (STRNEQ_NULLABLE(olddev->info.alias, newdev->info.alias)) { virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", _("cannot modify network device alias")); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index e10127d..fcb2deb 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -316,8 +316,8 @@ qemuMigrationCookieGraphicsAlloc(virQEMUDriverPtr driver, goto error; #endif } - if (!(mig->listen = strdup(listenAddr))) - goto no_memory; + if (VIR_STRDUP(mig->listen, listenAddr) < 0) + goto error; virObjectUnref(cfg); return mig; @@ -400,8 +400,8 @@ qemuMigrationCookieNew(virDomainObjPtr dom) name = priv->origname; else name = dom->def->name; - if (!(mig->name = strdup(name))) - goto no_memory; + if (VIR_STRDUP(mig->name, name) < 0) + goto error; memcpy(mig->uuid, dom->def->uuid, VIR_UUID_BUFLEN); if (!(mig->localHostname = virGetHostname())) @@ -460,15 +460,14 @@ qemuMigrationCookieAddLockstate(qemuMigrationCookiePtr mig, } if (virDomainObjGetState(dom, NULL) == VIR_DOMAIN_PAUSED) { - if (priv->lockState && - !(mig->lockState = strdup(priv->lockState))) + if (VIR_STRDUP(mig->lockState, priv->lockState) < 0) return -1; } else { if (virDomainLockProcessInquire(driver->lockManager, dom, &mig->lockState) < 0) return -1; } - if (!(mig->lockDriver = strdup(virLockManagerPluginGetName(driver->lockManager)))) { + if (VIR_STRDUP(mig->lockDriver, virLockManagerPluginGetName(driver->lockManager)) < 0) { VIR_FREE(mig->lockState); return -1; } @@ -2081,10 +2080,8 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver, /* QEMU will be started with -incoming stdio * (which qemu_command might convert to exec:cat or fd:n) */ - if (!(migrateFrom = strdup("stdio"))) { - virReportOOMError(); + if (VIR_STRDUP(migrateFrom, "stdio") < 0) goto cleanup; - } } else { virQEMUCapsPtr qemuCaps = NULL; struct addrinfo *info = NULL; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index d95198b..12657e5 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -201,11 +201,9 @@ int qemuMonitorJSONIOProcess(qemuMonitorPtr mon, if (nl) { int got = nl - (data + used); - char *line = strndup(data + used, got); - if (!line) { - virReportOOMError(); + char *line; + if (VIR_STRNDUP(line, data + used, got) < 0) return -1; - } used += got + strlen(LINE_ENDING); line[got] = '\0'; /* kill \n */ if (qemuMonitorJSONIOProcessLine(mon, line, msg) < 0) { @@ -958,15 +956,9 @@ qemuMonitorJSONHumanCommandWithFd(qemuMonitorPtr mon, if (reply_str) { const char *data; - if ((data = virJSONValueGetString(obj))) - *reply_str = strdup(data); - else - *reply_str = strdup(""); - - if (!*reply_str) { - virReportOOMError(); + data = virJSONValueGetString(obj); + if (VIR_STRDUP(*reply_str, data ? data : "") < 0) goto cleanup; - } } ret = 0; @@ -2946,11 +2938,9 @@ static int qemuMonitorJSONExtractPtyPaths(virJSONValuePtr reply, } if (STRPREFIX(type, "pty:")) { - char *path = strdup(type + strlen("pty:")); - if (!path) { - virReportOOMError(); + char *path; + if (VIR_STRDUP(path, type + strlen("pty:")) < 0) goto cleanup; - } if (virHashAddEntry(paths, id, path) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, @@ -3925,10 +3915,8 @@ int qemuMonitorJSONGetVersion(qemuMonitorPtr mon, _("query-version reply was missing 'package' version")); goto cleanup; } - if (!(*package = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(*package, tmp) < 0) goto cleanup; - } } ret = 0; @@ -4002,10 +3990,8 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, goto cleanup; } - if (!(info->name = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(info->name, tmp) < 0) goto cleanup; - } if (virJSONValueObjectHasKey(child, "is-default") && virJSONValueObjectGetBoolean(child, "is-default", &info->isDefault) < 0) { @@ -4020,10 +4006,8 @@ int qemuMonitorJSONGetMachines(qemuMonitorPtr mon, _("query-machines reply has malformed 'alias' data")); goto cleanup; } - if (!(info->alias = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(info->alias, tmp) < 0) goto cleanup; - } } } @@ -4108,10 +4092,8 @@ int qemuMonitorJSONGetCPUDefinitions(qemuMonitorPtr mon, goto cleanup; } - if (!(cpulist[i] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(cpulist[i], tmp) < 0) goto cleanup; - } } ret = n; @@ -4180,10 +4162,8 @@ int qemuMonitorJSONGetCommands(qemuMonitorPtr mon, goto cleanup; } - if (!(commandlist[i] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(commandlist[i], tmp) < 0) goto cleanup; - } } ret = n; @@ -4257,10 +4237,8 @@ int qemuMonitorJSONGetEvents(qemuMonitorPtr mon, goto cleanup; } - if (!(eventlist[i] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(eventlist[i], tmp) < 0) goto cleanup; - } } ret = n; @@ -4500,10 +4478,8 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitorPtr mon, goto cleanup; } - if (!(typelist[i] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(typelist[i], tmp) < 0) goto cleanup; - } } ret = n; @@ -4580,10 +4556,8 @@ int qemuMonitorJSONGetObjectProps(qemuMonitorPtr mon, goto cleanup; } - if (!(proplist[i] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(proplist[i], tmp) < 0) goto cleanup; - } } ret = n; @@ -4631,10 +4605,7 @@ qemuMonitorJSONGetTargetArch(qemuMonitorPtr mon) goto cleanup; } - if (!(ret = strdup(arch))) { - virReportOOMError(); - goto cleanup; - } + ignore_value(VIR_STRDUP(ret, arch)); cleanup: virJSONValueFree(cmd); @@ -4928,10 +4899,8 @@ qemuMonitorJSONGetStringArray(qemuMonitorPtr mon, const char *qmpCmd, goto cleanup; } - if (!(list[i] = strdup(tmp))) { - virReportOOMError(); + if (VIR_STRDUP(list[i], tmp) < 0) goto cleanup; - } } ret = n; diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c index 696961b..acc8a12 100644 --- a/src/qemu/qemu_monitor_text.c +++ b/src/qemu/qemu_monitor_text.c @@ -258,11 +258,8 @@ qemuMonitorTextCommandWithHandler(qemuMonitorPtr mon, if (msg.rxBuffer) { *reply = msg.rxBuffer; } else { - *reply = strdup(""); - if (!*reply) { - virReportOOMError(); + if (VIR_STRDUP(*reply, "") < 0) return -1; - } } } @@ -330,10 +327,8 @@ qemuMonitorSendDiskPassphrase(qemuMonitorPtr mon, /* Extra the path */ pathStart += strlen(DISK_ENCRYPTION_PREFIX); - if (!(path = strndup(pathStart, pathEnd - pathStart))) { - virReportOOMError(); + if (VIR_STRNDUP(path, pathStart, pathEnd - pathStart) < 0) return -1; - } /* Fetch the disk password if possible */ res = qemuMonitorGetDiskSecret(mon, @@ -2283,11 +2278,9 @@ int qemuMonitorTextGetPtyPaths(qemuMonitorPtr mon, /* Path is everything after needle to the end of the line */ *eol = '\0'; - char *path = strdup(needle + strlen(NEEDLE)); - if (path == NULL) { - virReportOOMError(); + char *path; + if (VIR_STRDUP(path, needle + strlen(NEEDLE)) < 0) goto cleanup; - } if (virHashAddEntry(paths, id, path) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a68601b..b6c1284 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1000,16 +1000,16 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (VIR_ALLOC(localAddr) < 0) goto no_memory; localAddr->family = localFamily; - if (!(localAddr->service = strdup(localService)) || - !(localAddr->node = strdup(localNode))) - goto no_memory; + if (VIR_STRDUP(localAddr->service, localService) < 0 || + VIR_STRDUP(localAddr->node, localNode) < 0) + goto error; if (VIR_ALLOC(remoteAddr) < 0) goto no_memory; remoteAddr->family = remoteFamily; - if (!(remoteAddr->service = strdup(remoteService)) || - !(remoteAddr->node = strdup(remoteNode))) - goto no_memory; + if (VIR_STRDUP(remoteAddr->service, remoteService) < 0 || + VIR_STRDUP(remoteAddr->node, remoteNode) < 0) + goto error; if (VIR_ALLOC(subject) < 0) goto no_memory; @@ -1017,17 +1017,17 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, if (VIR_REALLOC_N(subject->identities, subject->nidentity+1) < 0) goto no_memory; subject->nidentity++; - if (!(subject->identities[subject->nidentity-1].type = strdup("x509dname")) || - !(subject->identities[subject->nidentity-1].name = strdup(x509dname))) - goto no_memory; + if (VIR_STRDUP(subject->identities[subject->nidentity-1].type, "x509dname") < 0 || + VIR_STRDUP(subject->identities[subject->nidentity-1].name, x509dname) < 0) + goto error; } if (saslUsername) { if (VIR_REALLOC_N(subject->identities, subject->nidentity+1) < 0) goto no_memory; subject->nidentity++; - if (!(subject->identities[subject->nidentity-1].type = strdup("saslUsername")) || - !(subject->identities[subject->nidentity-1].name = strdup(saslUsername))) - goto no_memory; + if (VIR_STRDUP(subject->identities[subject->nidentity-1].type, "saslUsername") < 0 || + VIR_STRDUP(subject->identities[subject->nidentity-1].name, saslUsername) < 0) + goto error; } virObjectLock(vm); @@ -1041,6 +1041,7 @@ qemuProcessHandleGraphics(qemuMonitorPtr mon ATTRIBUTE_UNUSED, no_memory: virReportOOMError(); +error: if (localAddr) { VIR_FREE(localAddr->service); VIR_FREE(localAddr->node); @@ -1480,11 +1481,8 @@ qemuProcessExtractTTYPath(const char *haystack, */ while (*tmp) { if (c_isspace(*tmp)) { - *path = strndup(dev, tmp-dev); - if (*path == NULL) { - virReportOOMError(); + if (VIR_STRNDUP(*path, dev, tmp - dev) < 0) return -1; - } /* ... now further update offset till we get EOL */ *offset = tmp - haystack; @@ -1539,12 +1537,8 @@ qemuProcessLookupPTYs(virDomainChrDefPtr *devices, } VIR_FREE(chr->source.data.file.path); - chr->source.data.file.path = strdup(path); - - if (chr->source.data.file.path == NULL) { - virReportOOMError(); + if (VIR_STRDUP(chr->source.data.file.path, path) < 0) return -1; - } } } @@ -2666,12 +2660,12 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) if (state == VIR_DOMAIN_PAUSED && running) { newState = VIR_DOMAIN_RUNNING; newReason = VIR_DOMAIN_RUNNING_UNPAUSED; - msg = strdup("was unpaused"); + ignore_value(VIR_STRDUP(msg, "was unpaused")); } else if (state == VIR_DOMAIN_RUNNING && !running) { if (reason == VIR_DOMAIN_PAUSED_SHUTTING_DOWN) { newState = VIR_DOMAIN_SHUTDOWN; newReason = VIR_DOMAIN_SHUTDOWN_UNKNOWN; - msg = strdup("shutdown"); + ignore_value(VIR_STRDUP(msg, "shutdown")); } else { newState = VIR_DOMAIN_PAUSED; newReason = reason; @@ -2681,7 +2675,7 @@ qemuProcessUpdateState(virQEMUDriverPtr driver, virDomainObjPtr vm) } else if (state == VIR_DOMAIN_SHUTOFF && running) { newState = VIR_DOMAIN_RUNNING; newReason = VIR_DOMAIN_RUNNING_BOOTED; - msg = strdup("finished booting"); + ignore_value(VIR_STRDUP(msg, "finished booting")); } if (newState != VIR_DOMAIN_NOSTATE) { @@ -3476,13 +3470,10 @@ int qemuProcessStart(virConnectPtr conn, goto cleanup; } graphics->listens[0].type = VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS; - if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC) - graphics->listens[0].address = strdup(cfg->vncListen); - else - graphics->listens[0].address = strdup(cfg->spiceListen); - if (!graphics->listens[0].address) { + if (VIR_STRDUP(graphics->listens[0].address, + graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ? + cfg->vncListen : cfg->spiceListen) < 0) { VIR_SHRINK_N(graphics->listens, graphics->nListens, 1); - virReportOOMError(); goto cleanup; } } @@ -4257,9 +4248,8 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, } VIR_FREE(priv->pidfile); - if (pidfile && - !(priv->pidfile = strdup(pidfile))) - goto no_memory; + if (VIR_STRDUP(priv->pidfile, pidfile) < 0) + goto cleanup; VIR_DEBUG("Detect security driver config"); sec_managers = virSecurityManagerGetNested(driver->securityManager); @@ -4280,11 +4270,11 @@ int qemuProcessAttach(virConnectPtr conn ATTRIBUTE_UNUSED, vm->def, vm->pid, seclabel) < 0) goto cleanup; - if (!(seclabeldef->model = strdup(model))) - goto no_memory; + if (VIR_STRDUP(seclabeldef->model, model) < 0) + goto cleanup; - if (!(seclabeldef->label = strdup(seclabel->label))) - goto no_memory; + if (VIR_STRDUP(seclabeldef->label, seclabel->label) < 0) + goto cleanup; VIR_FREE(seclabel); } -- 1.8.2.1

--- src/remote/remote_driver.c | 114 +++++++++++++++++---------------------------- 1 file changed, 44 insertions(+), 70 deletions(-) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 97be2a0..ed3467f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -368,8 +368,8 @@ remoteClientCloseFunc(virNetClientPtr client ATTRIBUTE_UNUSED, #define EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \ if (STRCASEEQ(var->name, ARG_NAME)) { \ VIR_FREE(ARG_VAR); \ - if (!(ARG_VAR = strdup(var->value))) \ - goto no_memory; \ + if (VIR_STRDUP(ARG_VAR, var->value) < 0) \ + goto failed; \ var->ignore = 1; \ continue; \ } @@ -497,24 +497,21 @@ doRemoteOpen(virConnectPtr conn, if (virAsprintf(&port, "%d", conn->uri->port) < 0) goto no_memory; } else if (transport == trans_tls) { - if (!(port = strdup(LIBVIRTD_TLS_PORT))) - goto no_memory; + if (VIR_STRDUP(port, LIBVIRTD_TLS_PORT) < 0) + goto failed; } else if (transport == trans_tcp) { - if (!(port = strdup(LIBVIRTD_TCP_PORT))) - goto no_memory; + if (VIR_STRDUP(port, LIBVIRTD_TCP_PORT) < 0) + goto failed; } /* Port not used for unix, ext., default for ssh */ - if (conn->uri && conn->uri->server) - priv->hostname = strdup(conn->uri->server); - else - priv->hostname = strdup("localhost"); - - if (!priv->hostname) - goto no_memory; + if (VIR_STRDUP(priv->hostname, + conn->uri && conn->uri->server ? + conn->uri->server : "localhost") < 0) + goto failed; if (conn->uri && conn->uri->user && - !(username = strdup(conn->uri->user))) - goto no_memory; + VIR_STRDUP(username, conn->uri->user) < 0) + goto failed; /* Get the variables from the query string. * Then we need to reconstruct the query string (because @@ -557,8 +554,8 @@ doRemoteOpen(virConnectPtr conn, (STREQ(conn->uri->scheme, "remote") || STRPREFIX(conn->uri->scheme, "remote+"))) { /* Allow remote serve to probe */ - if (!(name = strdup(""))) - goto no_memory; + if (VIR_STRDUP(name, "") < 0) + goto failed; } else { virURI tmpuri = { .scheme = conn->uri->scheme, @@ -587,8 +584,8 @@ doRemoteOpen(virConnectPtr conn, } } else { /* Probe URI server side */ - if (!(name = strdup(""))) - goto no_memory; + if (VIR_STRDUP(name, "") < 0) + goto failed; } VIR_DEBUG("proceeding with name = %s", name); @@ -635,15 +632,11 @@ doRemoteOpen(virConnectPtr conn, break; case trans_libssh2: - if (!sockname) { - if (flags & VIR_DRV_OPEN_REMOTE_RO) - sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET_RO); - else - sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET); - - if (sockname == NULL) - goto no_memory; - } + if (!sockname && + VIR_STRDUP(sockname, + flags & VIR_DRV_OPEN_REMOTE_RO ? + LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + goto failed; VIR_DEBUG("Starting LibSSH2 session"); @@ -678,12 +671,10 @@ doRemoteOpen(virConnectPtr conn, } VIR_FREE(userdir); } else { - if (flags & VIR_DRV_OPEN_REMOTE_RO) - sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET_RO); - else - sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET); - if (sockname == NULL) - goto no_memory; + if (VIR_STRDUP(sockname, + flags & VIR_DRV_OPEN_REMOTE_RO ? + LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + goto failed; } VIR_DEBUG("Proceeding with sockname %s", sockname); } @@ -705,17 +696,13 @@ doRemoteOpen(virConnectPtr conn, break; case trans_ssh: - if (!command && !(command = strdup("ssh"))) - goto no_memory; + if (!command && VIR_STRDUP(command, "ssh") < 0) + goto failed; - if (!sockname) { - if (flags & VIR_DRV_OPEN_REMOTE_RO) - sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET_RO); - else - sockname = strdup(LIBVIRTD_PRIV_UNIX_SOCKET); - if (!sockname) - goto no_memory; - } + if (VIR_STRDUP(sockname, + flags & VIR_DRV_OPEN_REMOTE_RO ? + LIBVIRTD_PRIV_UNIX_SOCKET_RO : LIBVIRTD_PRIV_UNIX_SOCKET) < 0) + goto failed; if (!(priv->client = virNetClientNewSSH(priv->hostname, port, @@ -1493,11 +1480,8 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, for (i = 0; i < nparams; ++i) { /* call() will free this: */ - val[i].field = strdup(params[i].field); - if (val[i].field == NULL) { - virReportOOMError(); + if (VIR_STRDUP(val[i].field, params[i].field) < 0) goto cleanup; - } val[i].value.type = params[i].type; switch (params[i].type) { case VIR_TYPED_PARAM_INT: @@ -1519,11 +1503,9 @@ remoteSerializeTypedParameters(virTypedParameterPtr params, val[i].value.remote_typed_param_value_u.b = params[i].value.b; break; case VIR_TYPED_PARAM_STRING: - val[i].value.remote_typed_param_value_u.s = strdup(params[i].value.s); - if (val[i].value.remote_typed_param_value_u.s == NULL) { - virReportOOMError(); + if (VIR_STRDUP(val[i].value.remote_typed_param_value_u.s, + params[i].value.s) < 0) goto cleanup; - } break; default: virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), @@ -1608,12 +1590,9 @@ remoteDeserializeTypedParameters(remote_typed_param *ret_params_val, ret_param->value.remote_typed_param_value_u.b; break; case VIR_TYPED_PARAM_STRING: - param->value.s = - strdup(ret_param->value.remote_typed_param_value_u.s); - if (!param->value.s) { - virReportOOMError(); + if (VIR_STRDUP(param->value.s, + ret_param->value.remote_typed_param_value_u.s) < 0) goto cleanup; - } break; default: virReportError(VIR_ERR_RPC, _("unknown parameter type: %d"), @@ -1653,10 +1632,8 @@ remoteDeserializeDomainDiskErrors(remote_domain_disk_error *ret_errors_val, } for (i = 0; i < ret_errors_len; i++) { - if (!(errors[i].disk = strdup(ret_errors_val[i].disk))) { - virReportOOMError(); + if (VIR_STRDUP(errors[i].disk, ret_errors_val[i].disk) < 0) goto error; - } errors[i].error = ret_errors_val[i].error; } @@ -4472,15 +4449,15 @@ remoteDomainBuildEventGraphics(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, if (VIR_ALLOC(localAddr) < 0) goto no_memory; localAddr->family = msg->local.family; - if (!(localAddr->service = strdup(msg->local.service)) || - !(localAddr->node = strdup(msg->local.node))) + if (VIR_STRDUP(localAddr->service, msg->local.service) < 0 || + VIR_STRDUP(localAddr->node, msg->local.node) < 0) goto no_memory; if (VIR_ALLOC(remoteAddr) < 0) goto no_memory; remoteAddr->family = msg->remote.family; - if (!(remoteAddr->service = strdup(msg->remote.service)) || - !(remoteAddr->node = strdup(msg->remote.node))) + if (VIR_STRDUP(remoteAddr->service, msg->remote.service) < 0 || + VIR_STRDUP(remoteAddr->node, msg->remote.node) < 0) goto no_memory; if (VIR_ALLOC(subject) < 0) @@ -4489,8 +4466,8 @@ remoteDomainBuildEventGraphics(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, goto no_memory; subject->nidentity = msg->subject.subject_len; for (i = 0 ; i < subject->nidentity ; i++) { - if (!(subject->identities[i].type = strdup(msg->subject.subject_val[i].type)) || - !(subject->identities[i].name = strdup(msg->subject.subject_val[i].name))) + if (VIR_STRDUP(subject->identities[i].type, msg->subject.subject_val[i].type) < 0 || + VIR_STRDUP(subject->identities[i].name, msg->subject.subject_val[i].name) < 0) goto no_memory; } @@ -5100,11 +5077,8 @@ remoteDomainQemuMonitorCommand(virDomainPtr domain, const char *cmd, (xdrproc_t) xdr_qemu_domain_monitor_command_ret, (char *) &ret) == -1) goto done; - *result = strdup(ret.result); - if (*result == NULL) { - virReportOOMError(); + if (VIR_STRDUP(*result, ret.result) < 0) goto cleanup; - } rv = 0; -- 1.8.2.1

--- src/rpc/gendispatch.pl | 21 ++++-------- src/rpc/virnetclient.c | 16 ++++----- src/rpc/virnetmessage.c | 27 +++++++++------ src/rpc/virnetsaslcontext.c | 6 ++-- src/rpc/virnetserver.c | 6 ++-- src/rpc/virnetserverclient.c | 10 ++---- src/rpc/virnetservermdns.c | 6 ++-- src/rpc/virnetsocket.c | 10 +++--- src/rpc/virnetsshsession.c | 78 +++++++++++++++++++++----------------------- src/rpc/virnettlscontext.c | 26 +++++++-------- 10 files changed, 92 insertions(+), 114 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 8d3b013..8c83cd2 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -610,12 +610,10 @@ elsif ($mode eq "server") { # SPECIAL: virConnectGetType returns a constant string that must # not be freed. Therefore, duplicate the string here. push(@vars_list, "const char *$1"); - push(@ret_list, "/* We have to strdup because remoteDispatchClientRequest will"); + push(@ret_list, "/* We have to VIR_STRDUP because remoteDispatchClientRequest will"); push(@ret_list, " * free this string after it's been serialised. */"); - push(@ret_list, "if (!(ret->type = strdup(type))) {"); - push(@ret_list, " virReportOOMError();"); + push(@ret_list, "if (VIR_STRDUP(ret->type, type) < 0)"); push(@ret_list, " goto cleanup;"); - push(@ret_list, "}"); } else { push(@vars_list, "char *$1"); push(@ret_list, "ret->$1 = $1;"); @@ -636,11 +634,8 @@ elsif ($mode eq "server") { " goto cleanup;\n" . " }\n" . " \n" . - " *$1_p = strdup($1);\n" . - " if (*$1_p == NULL) {\n" . - " virReportOOMError();\n" . - " goto cleanup;\n" . - " }\n"); + " if (VIR_STRDUP(*$1_p, $1) < 0)\n". + " goto cleanup;\n"); $single_ret_var = $1; $single_ret_by_ref = 0; @@ -1562,16 +1557,14 @@ elsif ($mode eq "client") { print "\n"; print " /* This call is caller-frees (although that isn't clear from\n"; print " * the documentation). However xdr_free will free up both the\n"; - print " * names and the list of pointers, so we have to strdup the\n"; + print " * names and the list of pointers, so we have to VIR_STRDUP the\n"; print " * names here. */\n"; print " for (i = 0; i < ret.$single_ret_list_name.${single_ret_list_name}_len; ++i) {\n"; - print " ${single_ret_list_name}[i] = strdup(ret.$single_ret_list_name.${single_ret_list_name}_val[i]);\n"; - print "\n"; - print " if (${single_ret_list_name}[i] == NULL) {\n"; + print " if (VIR_STRDUP(${single_ret_list_name}[i],\n"; + print " ret.$single_ret_list_name.${single_ret_list_name}_val[i]) < 0) {\n"; print " for (--i; i >= 0; --i)\n"; print " VIR_FREE(${single_ret_list_name}[i]);\n"; print "\n"; - print " virReportOOMError();\n"; print " goto cleanup;\n"; print " }\n"; print " }\n"; diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 1d228f0..7315fdf 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -36,6 +36,7 @@ #include "virlog.h" #include "virutil.h" #include "virerror.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_RPC @@ -317,17 +318,14 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock, client->wakeupSendFD = wakeupFD[1]; wakeupFD[0] = wakeupFD[1] = -1; - if (hostname && - !(client->hostname = strdup(hostname))) - goto no_memory; + if (VIR_STRDUP(client->hostname, hostname) < 0) + goto error; PROBE(RPC_CLIENT_NEW, "client=%p sock=%p", client, client->sock); return client; -no_memory: - virReportOOMError(); error: VIR_FORCE_CLOSE(wakeupFD[0]); VIR_FORCE_CLOSE(wakeupFD[1]); @@ -414,8 +412,8 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, goto no_memory; } } else { - if (!(knownhosts = strdup(knownHostsPath))) - goto no_memory; + if (VIR_STRDUP(knownhosts, knownHostsPath) < 0) + goto cleanup; } } @@ -438,8 +436,8 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, VIR_FREE(privkey); } } else { - if (!(privkey = strdup(privkeyPath))) - goto no_memory; + if (VIR_STRDUP(privkey, privkeyPath) < 0) + goto cleanup; } } diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index b2c6e5b..8483cd5 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -29,6 +29,7 @@ #include "virlog.h" #include "virfile.h" #include "virutil.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_RPC @@ -514,22 +515,28 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) if (verr) { rerr->code = verr->code; rerr->domain = verr->domain; - if (verr->message && VIR_ALLOC(rerr->message) == 0) - *rerr->message = strdup(verr->message); + if (verr->message && VIR_ALLOC(rerr->message) == 0 && + VIR_STRDUP_QUIET(*rerr->message, verr->message) < 0) + VIR_FREE(rerr->message); rerr->level = verr->level; - if (verr->str1 && VIR_ALLOC(rerr->str1) == 0) - *rerr->str1 = strdup(verr->str1); - if (verr->str2 && VIR_ALLOC(rerr->str2) == 0) - *rerr->str2 = strdup(verr->str2); - if (verr->str3 && VIR_ALLOC(rerr->str3) == 0) - *rerr->str3 = strdup(verr->str3); + if (verr->str1 && VIR_ALLOC(rerr->str1) == 0 && + VIR_STRDUP_QUIET(*rerr->str1, verr->str1) < 0) + VIR_FREE(verr->str1); + if (verr->str2 && VIR_ALLOC(rerr->str2) == 0 && + VIR_STRDUP_QUIET(*rerr->str2, verr->str2) < 0) + VIR_FREE(verr->str2); + if (verr->str3 && VIR_ALLOC(rerr->str3) == 0 && + VIR_STRDUP_QUIET(*rerr->str3, verr->str3) < 0) + VIR_FREE(verr->str2); rerr->int1 = verr->int1; rerr->int2 = verr->int2; } else { rerr->code = VIR_ERR_INTERNAL_ERROR; rerr->domain = VIR_FROM_RPC; - if (VIR_ALLOC(rerr->message) == 0) - *rerr->message = strdup(_("Library function returned error but did not set virError")); + if (VIR_ALLOC(rerr->message) == 0 && + VIR_STRDUP_QUIET(*rerr->message, + _("Library function returned error but did not set virError")) < 0) + VIR_FREE(rerr->message); rerr->level = VIR_ERR_ERROR; } } diff --git a/src/rpc/virnetsaslcontext.c b/src/rpc/virnetsaslcontext.c index 6943216..35dc6cf 100644 --- a/src/rpc/virnetsaslcontext.c +++ b/src/rpc/virnetsaslcontext.c @@ -29,6 +29,7 @@ #include "viralloc.h" #include "virthread.h" #include "virlog.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_RPC @@ -385,10 +386,7 @@ char *virNetSASLSessionListMechanisms(virNetSASLSessionPtr sasl) err, sasl_errdetail(sasl->conn)); goto cleanup; } - if (!(ret = strdup(mechlist))) { - virReportOOMError(); - goto cleanup; - } + ignore_value(VIR_STRDUP(ret, mechlist)); cleanup: virObjectUnlock(sasl); diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index e536cc3..eb2fb3e 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -37,6 +37,7 @@ #include "virfile.h" #include "virnetservermdns.h" #include "virdbus.h" +#include "virstring.h" #ifndef SA_SIGINFO # define SA_SIGINFO 0 @@ -387,11 +388,8 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->privileged = geteuid() == 0; srv->autoShutdownInhibitFd = -1; - if (mdnsGroupName && - !(srv->mdnsGroupName = strdup(mdnsGroupName))) { - virReportOOMError(); + if (VIR_STRDUP(srv->mdnsGroupName, mdnsGroupName) < 0) goto error; - } if (srv->mdnsGroupName) { if (!(srv->mdns = virNetServerMDNSNew())) goto error; diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 6d7c439..f339259 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -693,22 +693,16 @@ virNetServerClientCreateIdentity(virNetServerClientPtr client) #if WITH_SASL if (client->sasl) { const char *identity = virNetSASLSessionGetIdentity(client->sasl); - if (identity && - !(saslname = strdup(identity))) { - virReportOOMError(); + if (VIR_STRDUP(saslname, identity) < 0) goto cleanup; - } } #endif #if WITH_GNUTLS if (client->tls) { const char *identity = virNetTLSSessionGetX509DName(client->tls); - if (identity && - !(x509dname = strdup(identity))) { - virReportOOMError(); + if (VIR_STRDUP(x509dname, identity) < 0) goto cleanup; - } } #endif diff --git a/src/rpc/virnetservermdns.c b/src/rpc/virnetservermdns.c index 26e24d5..68d5e4e 100644 --- a/src/rpc/virnetservermdns.c +++ b/src/rpc/virnetservermdns.c @@ -479,9 +479,8 @@ virNetServerMDNSGroupPtr virNetServerMDNSAddGroup(virNetServerMDNS *mdns, return NULL; } - if (!(group->name = strdup(name))) { + if (VIR_STRDUP(group->name, name) < 0) { VIR_FREE(group); - virReportOOMError(); return NULL; } group->mdns = mdns; @@ -525,9 +524,8 @@ virNetServerMDNSEntryPtr virNetServerMDNSAddEntry(virNetServerMDNSGroupPtr group } entry->port = port; - if (!(entry->type = strdup(type))) { + if (VIR_STRDUP(entry->type, type) < 0) { VIR_FREE(entry); - virReportOOMError(); return NULL; } entry->next = group->entry; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 67eb496..c0c81d0 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -800,10 +800,10 @@ virNetSocketNewConnectLibSSH2(const char *host, if (virNetSSHSessionSetChannelCommand(sess, command) != 0) goto error; - if (!(authMethodNext = authMethodsCopy = strdup(authMethods))) { - virReportOOMError(); + if (VIR_STRDUP(authMethodsCopy, authMethods) < 0) goto error; - } + + authMethodNext = authMethodsCopy; while ((authMethod = strsep(&authMethodNext, ","))) { if (STRCASEEQ(authMethod, "keyboard-interactive")) @@ -1191,10 +1191,8 @@ int virNetSocketGetSELinuxContext(virNetSocketPtr sock, goto cleanup; } - if (!(*context = strdup(seccon))) { - virReportOOMError(); + if (VIR_STRDUP(*context, seccon) < 0) goto cleanup; - } ret = 0; cleanup: diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index 189d928..c28a395 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -234,7 +234,7 @@ virNetSSHKbIntCb(const char *name ATTRIBUTE_UNUSED, /* fill data structures for auth callback */ for (i = 0; i < num_prompts; i++) { - if (!(askcred[i].prompt = strdup(prompts[i].text))) { + if (VIR_STRDUP(askcred[i].prompt, prompts[i].text) < 0) { priv->authCbErr = VIR_NET_SSH_AUTHCB_OOM; goto cleanup; } @@ -739,7 +739,7 @@ virNetSSHAuthenticateKeyboardInteractive(virNetSSHSessionPtr sess, "authentication credentials")); return -1; case VIR_NET_SSH_AUTHCB_OOM: - virReportOOMError(); + /* OOM error already reported */ return -1; case VIR_NET_SSH_AUTHCB_RETR_ERR: virReportError(VIR_ERR_SSH, "%s", @@ -960,12 +960,14 @@ virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, virObjectLock(sess); - if (!(user = strdup(username)) || - !(pass = strdup(password))) - goto no_memory; + if (VIR_STRDUP(user, username) < 0 || + VIR_STRDUP(pass, password) < 0) + goto error; - if (!(auth = virNetSSHSessionAuthMethodNew(sess))) - goto no_memory; + if (!(auth = virNetSSHSessionAuthMethodNew(sess))) { + virReportOOMError(); + goto error; + } auth->username = user; auth->password = pass; @@ -974,10 +976,9 @@ virNetSSHSessionAuthAddPasswordAuth(virNetSSHSessionPtr sess, virObjectUnlock(sess); return 0; -no_memory: +error: VIR_FREE(user); VIR_FREE(pass); - virReportOOMError(); virObjectUnlock(sess); return -1; } @@ -998,11 +999,13 @@ virNetSSHSessionAuthAddAgentAuth(virNetSSHSessionPtr sess, virObjectLock(sess); - if (!(user = strdup(username))) - goto no_memory; + if (VIR_STRDUP(user, username) < 0) + goto error; - if (!(auth = virNetSSHSessionAuthMethodNew(sess))) - goto no_memory; + if (!(auth = virNetSSHSessionAuthMethodNew(sess))) { + virReportOOMError(); + goto error; + } auth->username = user; auth->method = VIR_NET_SSH_AUTH_AGENT; @@ -1010,9 +1013,8 @@ virNetSSHSessionAuthAddAgentAuth(virNetSSHSessionPtr sess, virObjectUnlock(sess); return 0; -no_memory: +error: VIR_FREE(user); - virReportOOMError(); virObjectUnlock(sess); return -1; } @@ -1038,15 +1040,15 @@ virNetSSHSessionAuthAddPrivKeyAuth(virNetSSHSessionPtr sess, virObjectLock(sess); - if (!(user = strdup(username)) || - !(file = strdup(keyfile))) - goto no_memory; + if (VIR_STRDUP(user, username) < 0 || + VIR_STRDUP(file, keyfile) < 0 || + VIR_STRDUP(pass, password) < 0) + goto error; - if (password && !(pass = strdup(password))) - goto no_memory; - - if (!(auth = virNetSSHSessionAuthMethodNew(sess))) - goto no_memory; + if (!(auth = virNetSSHSessionAuthMethodNew(sess))) { + virReportOOMError(); + goto error; + } auth->username = user; auth->password = pass; @@ -1056,11 +1058,10 @@ virNetSSHSessionAuthAddPrivKeyAuth(virNetSSHSessionPtr sess, virObjectUnlock(sess); return 0; -no_memory: +error: VIR_FREE(user); VIR_FREE(pass); VIR_FREE(file); - virReportOOMError(); virObjectUnlock(sess); return -1; } @@ -1082,11 +1083,13 @@ virNetSSHSessionAuthAddKeyboardAuth(virNetSSHSessionPtr sess, virObjectLock(sess); - if (!(user = strdup(username))) - goto no_memory; + if (VIR_STRDUP(user, username) < 0) + goto error; - if (!(auth = virNetSSHSessionAuthMethodNew(sess))) - goto no_memory; + if (!(auth = virNetSSHSessionAuthMethodNew(sess))) { + virReportOOMError(); + goto error; + } auth->username = user; auth->tries = tries; @@ -1095,9 +1098,8 @@ virNetSSHSessionAuthAddKeyboardAuth(virNetSSHSessionPtr sess, virObjectUnlock(sess); return 0; -no_memory: +error: VIR_FREE(user); - virReportOOMError(); virObjectUnlock(sess); return -1; @@ -1112,10 +1114,8 @@ virNetSSHSessionSetChannelCommand(virNetSSHSessionPtr sess, VIR_FREE(sess->channelCommand); - if (command && !(sess->channelCommand = strdup(command))) { - virReportOOMError(); + if (VIR_STRDUP(sess->channelCommand, command) < 0) ret = -1; - } virObjectUnlock(sess); return ret; @@ -1138,8 +1138,8 @@ virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, VIR_FREE(sess->hostname); - if (hostname && !(sess->hostname = strdup(hostname))) - goto no_memory; + if (VIR_STRDUP(sess->hostname, hostname) < 0) + goto error; /* load the known hosts file */ if (hostsfile) { @@ -1163,16 +1163,14 @@ virNetSSHSessionSetHostKeyVerification(virNetSSHSessionPtr sess, /* set filename only if writing to the known hosts file is requested */ if (!(flags & VIR_NET_SSH_HOSTKEY_FILE_READONLY)) { VIR_FREE(sess->knownHostsFile); - if (!(sess->knownHostsFile = strdup(hostsfile))) - goto no_memory; + if (VIR_STRDUP(sess->knownHostsFile, hostsfile) < 0) + goto error; } } virObjectUnlock(sess); return 0; -no_memory: - virReportOOMError(); error: virObjectUnlock(sess); return -1; diff --git a/src/rpc/virnettlscontext.c b/src/rpc/virnettlscontext.c index 1a7ccb8..4a31ff6 100644 --- a/src/rpc/virnettlscontext.c +++ b/src/rpc/virnettlscontext.c @@ -837,23 +837,23 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, */ if (!*cacert) { VIR_DEBUG("Using default TLS CA certificate path"); - if (!(*cacert = strdup(LIBVIRT_CACERT))) - goto out_of_memory; + if (VIR_STRDUP(*cacert, LIBVIRT_CACERT) < 0) + goto error; } if (!*cacrl) { VIR_DEBUG("Using default TLS CA revocation list path"); - if (!(*cacrl = strdup(LIBVIRT_CACRL))) - goto out_of_memory; + if (VIR_STRDUP(*cacrl, LIBVIRT_CACRL) < 0) + goto error; } if (!*key && !*cert) { VIR_DEBUG("Using default TLS key/certificate path"); - if (!(*key = strdup(isServer ? LIBVIRT_SERVERKEY : LIBVIRT_CLIENTKEY))) - goto out_of_memory; + if (VIR_STRDUP(*key, isServer ? LIBVIRT_SERVERKEY : LIBVIRT_CLIENTKEY) < 0) + goto error; - if (!(*cert = strdup(isServer ? LIBVIRT_SERVERCERT : LIBVIRT_CLIENTCERT))) - goto out_of_memory; + if (VIR_STRDUP(*cert, isServer ? LIBVIRT_SERVERCERT : LIBVIRT_CLIENTCERT) < 0) + goto error; } VIR_FREE(user_pki_path); @@ -863,6 +863,7 @@ static int virNetTLSContextLocateCredentials(const char *pkipath, out_of_memory: virReportOOMError(); +error: VIR_FREE(*cacert); VIR_FREE(*cacrl); VIR_FREE(*key); @@ -1029,10 +1030,8 @@ static int virNetTLSContextValidCertificate(virNetTLSContextPtr ctxt, "[session]", gnutls_strerror(ret)); goto authfail; } - if (!(sess->x509dname = strdup(dname))) { - virReportOOMError(); + if (VIR_STRDUP(sess->x509dname, dname) < 0) goto authfail; - } VIR_DEBUG("Peer DN is %s", dname); if (virNetTLSContextCheckCertDN(cert, "[session]", sess->hostname, dname, @@ -1169,11 +1168,8 @@ virNetTLSSessionPtr virNetTLSSessionNew(virNetTLSContextPtr ctxt, if (!(sess = virObjectLockableNew(virNetTLSSessionClass))) return NULL; - if (hostname && - !(sess->hostname = strdup(hostname))) { - virReportOOMError(); + if (VIR_STRDUP(sess->hostname, hostname) < 0) goto error; - } if ((err = gnutls_init(&sess->session, ctxt->isServer ? GNUTLS_SERVER : GNUTLS_CLIENT)) != 0) { -- 1.8.2.1

On 23.05.2013 00:26, Eric Blake wrote: Since 04/13 got NACKed, I needed to squash this in before pushing. diff --git a/src/rpc/virnetsshsession.c b/src/rpc/virnetsshsession.c index c28a395..b6aedc8 100644 --- a/src/rpc/virnetsshsession.c +++ b/src/rpc/virnetsshsession.c @@ -234,10 +234,12 @@ virNetSSHKbIntCb(const char *name ATTRIBUTE_UNUSED, /* fill data structures for auth callback */ for (i = 0; i < num_prompts; i++) { - if (VIR_STRDUP(askcred[i].prompt, prompts[i].text) < 0) { + char *prompt; + if (VIR_STRDUP(prompt, prompts[i].text) < 0) { priv->authCbErr = VIR_NET_SSH_AUTHCB_OOM; goto cleanup; } + askcred[i].prompt = prompt; /* remove colon and trailing spaces from prompts, as default behavior * of libvirt's auth callback is to add them */ Michal

--- src/security/security_apparmor.c | 20 ++-------- src/security/security_dac.c | 21 +++-------- src/security/security_nop.c | 7 +--- src/security/security_selinux.c | 79 +++++++++++----------------------------- src/security/virt-aa-helper.c | 4 +- 5 files changed, 35 insertions(+), 96 deletions(-) diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c index 3680279..87c2777 100644 --- a/src/security/security_apparmor.c +++ b/src/security/security_apparmor.c @@ -445,24 +445,15 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, if ((profile_name = get_profile_name(def)) == NULL) return rc; - secdef->label = strndup(profile_name, strlen(profile_name)); - if (!secdef->label) { - virReportOOMError(); + if (VIR_STRDUP(secdef->label, profile_name) < 0) goto cleanup; - } /* set imagelabel the same as label (but we won't use it) */ - secdef->imagelabel = strndup(profile_name, - strlen(profile_name)); - if (!secdef->imagelabel) { - virReportOOMError(); + if (VIR_STRDUP(secdef->imagelabel, profile_name) < 0) goto err; - } - if (!secdef->model && !(secdef->model = strdup(SECURITY_APPARMOR_NAME))) { - virReportOOMError(); + if (!secdef->model && VIR_STRDUP(secdef->model, SECURITY_APPARMOR_NAME) < 0) goto err; - } /* Now that we have a label, load the profile into the kernel. */ if (load_profile(mgr, secdef->label, def, NULL, false) < 0) { @@ -949,10 +940,7 @@ AppArmorGetMountOptions(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED, { char *opts; - if (!(opts = strdup(""))) { - virReportOOMError(); - return NULL; - } + ignore_value(VIR_STRDUP(opts, "")); return opts; } diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 6e6fcad..543ddeb 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -78,11 +78,8 @@ int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr) char *owner = NULL; char *group = NULL; - tmp_label = strdup(label); - if (tmp_label == NULL) { - virReportOOMError(); + if (VIR_STRDUP(tmp_label, label) < 0) goto cleanup; - } /* Split label */ sep = strchr(tmp_label, ':'); @@ -1104,18 +1101,10 @@ virSecurityDACGenLabel(virSecurityManagerPtr mgr, return rc; } - if (!seclabel->norelabel) { - if (seclabel->imagelabel == NULL && seclabel->label != NULL) { - seclabel->imagelabel = strdup(seclabel->label); - if (seclabel->imagelabel == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot generate dac user and group id " - "for domain %s"), def->name); - VIR_FREE(seclabel->label); - seclabel->label = NULL; - return rc; - } - } + if (!seclabel->norelabel && !seclabel->imagelabel && + VIR_STRDUP(seclabel->imagelabel, seclabel->label) < 0) { + VIR_FREE(seclabel->label); + return rc; } return 0; diff --git a/src/security/security_nop.c b/src/security/security_nop.c index 2b9767e..233404c 100644 --- a/src/security/security_nop.c +++ b/src/security/security_nop.c @@ -20,7 +20,7 @@ #include <config.h> #include "security_nop.h" - +#include "virstring.h" #include "virerror.h" #define VIR_FROM_THIS VIR_FROM_SECURITY @@ -182,10 +182,7 @@ static char *virSecurityDomainGetMountOptionsNop(virSecurityManagerPtr mgr ATTRI { char *opts; - if (!(opts = strdup(""))) { - virReportOOMError(); - return NULL; - } + ignore_value(VIR_STRDUP(opts, "")); return opts; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5d108b9..2d67652 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -203,10 +203,8 @@ virSecuritySELinuxMCSGetProcessRange(char **sens, goto cleanup; } - if (!(*sens = strdup(context_range_get(ourContext)))) { - virReportOOMError(); + if (VIR_STRDUP(*sens, context_range_get(ourContext)) < 0) goto cleanup; - } /* Find and blank out the category part (if any) */ tmp = strchr(*sens, ':'); @@ -313,10 +311,7 @@ virSecuritySELinuxContextAddRange(security_context_t src, goto cleanup; } - if (!(ret = strdup(str))) { - virReportOOMError(); - goto cleanup; - } + ignore_value(VIR_STRDUP(ret, str)); cleanup: if (srccon) context_free(srccon); @@ -386,10 +381,8 @@ virSecuritySELinuxGenNewContext(const char *basecontext, _("Unable to format SELinux context")); goto cleanup; } - if (!(ret = strdup(str))) { - virReportOOMError(); + if (VIR_STRDUP(ret, str) < 0) goto cleanup; - } VIR_DEBUG("Generated context '%s'", ret); cleanup: freecon(ourSecContext); @@ -452,17 +445,10 @@ virSecuritySELinuxLXCInitialize(virSecurityManagerPtr mgr) goto error; } - data->domain_context = strdup(scon->str); - data->file_context = strdup(tcon->str); - data->content_context = strdup(dcon->str); - if (!data->domain_context || - !data->file_context || - !data->content_context) { - virReportSystemError(errno, - _("cannot allocate memory for LXC SELinux contexts '%s'"), - selinux_lxc_contexts_path()); + if (VIR_STRDUP(data->domain_context, scon->str) < 0 || + VIR_STRDUP(data->file_context, tcon->str) < 0 || + VIR_STRDUP(data->content_context, dcon->str) < 0) goto error; - } if (!(data->mcs = virHashCreate(10, NULL))) goto error; @@ -521,11 +507,8 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) *ptr = '\0'; ptr++; if (*ptr != '\0') { - data->alt_domain_context = strdup(ptr); - if (!data->alt_domain_context) { - virReportOOMError(); + if (VIR_STRDUP(data->alt_domain_context, ptr) < 0) goto error; - } ptr = strchrnul(data->alt_domain_context, '\n'); if (ptr && *ptr == '\n') *ptr = '\0'; @@ -545,11 +528,8 @@ virSecuritySELinuxQEMUInitialize(virSecurityManagerPtr mgr) ptr = strchrnul(data->file_context, '\n'); if (ptr && *ptr == '\n') { *ptr = '\0'; - data->content_context = strdup(ptr+1); - if (!data->content_context) { - virReportOOMError(); + if (VIR_STRDUP(data->content_context, ptr + 1) < 0) goto error; - } ptr = strchrnul(data->content_context, '\n'); if (ptr && *ptr == '\n') *ptr = '\0'; @@ -644,11 +624,12 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } range = context_range_get(ctx); - if (!range || - !(mcs = strdup(range))) { + if (!range) { virReportOOMError(); goto cleanup; } + if (VIR_STRDUP(mcs, range) < 0) + goto cleanup; break; case VIR_DOMAIN_SECLABEL_DYNAMIC: @@ -712,10 +693,8 @@ virSecuritySELinuxGenSecurityLabel(virSecurityManagerPtr mgr, } if (!seclabel->model && - !(seclabel->model = strdup(SECURITY_SELINUX_NAME))) { - virReportOOMError(); + VIR_STRDUP(seclabel->model, SECURITY_SELINUX_NAME) < 0) goto cleanup; - } rc = 0; @@ -1413,10 +1392,8 @@ virSecuritySELinuxSetSecurityHostdevCapsLabel(virDomainDefPtr def, return -1; } } else { - if (!(path = strdup(dev->source.caps.u.storage.block))) { - virReportOOMError(); + if (VIR_STRDUP(path, dev->source.caps.u.storage.block) < 0) return -1; - } } ret = virSecuritySELinuxSetFilecon(path, secdef->imagelabel); VIR_FREE(path); @@ -1431,10 +1408,8 @@ virSecuritySELinuxSetSecurityHostdevCapsLabel(virDomainDefPtr def, return -1; } } else { - if (!(path = strdup(dev->source.caps.u.misc.chardev))) { - virReportOOMError(); + if (VIR_STRDUP(path, dev->source.caps.u.misc.chardev) < 0) return -1; - } } ret = virSecuritySELinuxSetFilecon(path, secdef->imagelabel); VIR_FREE(path); @@ -1607,10 +1582,8 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virSecurityManagerPtr mgr, return -1; } } else { - if (!(path = strdup(dev->source.caps.u.storage.block))) { - virReportOOMError(); + if (VIR_STRDUP(path, dev->source.caps.u.storage.block) < 0) return -1; - } } ret = virSecuritySELinuxRestoreSecurityFileLabel(mgr, path); VIR_FREE(path); @@ -1625,10 +1598,8 @@ virSecuritySELinuxRestoreSecurityHostdevCapsLabel(virSecurityManagerPtr mgr, return -1; } } else { - if (!(path = strdup(dev->source.caps.u.misc.chardev))) { - virReportOOMError(); + if (VIR_STRDUP(path, dev->source.caps.u.misc.chardev) < 0) return -1; - } } ret = virSecuritySELinuxRestoreSecurityFileLabel(mgr, path); VIR_FREE(path); @@ -2414,7 +2385,7 @@ virSecuritySELinuxGenImageLabel(virSecurityManagerPtr mgr, const char *range; context_t ctx = NULL; char *label = NULL; - const char *mcs = NULL; + char *mcs = NULL; secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); if (secdef == NULL) @@ -2428,11 +2399,8 @@ virSecuritySELinuxGenImageLabel(virSecurityManagerPtr mgr, } range = context_range_get(ctx); if (range) { - mcs = strdup(range); - if (!mcs) { - virReportOOMError(); + if (VIR_STRDUP(mcs, range) < 0) goto cleanup; - } if (!(label = virSecuritySELinuxGenNewContext(data->file_context, mcs, true))) goto cleanup; @@ -2440,9 +2408,9 @@ virSecuritySELinuxGenImageLabel(virSecurityManagerPtr mgr, } cleanup: - context_free(ctx); - VIR_FREE(mcs); - return label; + context_free(ctx); + VIR_FREE(mcs); + return label; } static char * @@ -2465,11 +2433,8 @@ virSecuritySELinuxGetSecurityMountOptions(virSecurityManagerPtr mgr, } } - if (!opts && - !(opts = strdup(""))) { - virReportOOMError(); + if (!opts && VIR_STRDUP(opts, "") < 0) return NULL; - } VIR_DEBUG("imageLabel=%s opts=%s", secdef ? secdef->imagelabel : "(null)", opts); diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 5d48850..cee18f1 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -773,7 +773,7 @@ vah_add_file(virBufferPtr buf, const char *path, const char *perms) return rc; } } else - if ((tmp = strdup(path)) == NULL) + if (VIR_STRDUP_QUIET(tmp, path) < 0) return rc; if (strchr(perms, 'w') != NULL) @@ -1103,7 +1103,7 @@ vahParseArgv(vahControl * ctl, int argc, char **argv) break; case 'f': case 'F': - if ((ctl->newfile = strdup(optarg)) == NULL) + if (VIR_STRDUP_QUIET(ctl->newfile, optarg) < 0) vah_error(ctl, 1, _("could not allocate memory for disk")); ctl->append = arg == 'F'; break; -- 1.8.2.1

On 05/20/2013 11:55 AM, Michal Privoznik wrote: I've got my work cut out for me! Hmm, I've seen this three-line pattern (declare temp var, strdup "" into it, use the var) in several patches now. I think it might help to have a new function in virstring.h whose job in life is to return a malloc'd copy of an empty string, as a one-liner, so that callers don't have to mess with a temp var. And notice that it's slightly more efficient to just zero-initialize a malloc'd array of 1, instead of going through strdup machinery, when we know the output will be an empty string. Maybe: /* Return a malloc'd empty string, or NULL after reporting OOM */ char * virStringEmpty(void) { char *ret; // assuming we fix VIR_ALLOC to report oom... ignore_value(VIR_ALLOC(ret)); return ret; } then THIS code could use the shorter: if (cur < 0) return virStringEmpty(); But if you decide to go that route, it's probably worth a separate cleanup pass, so this commit is not delayed. double-oom, since this function was previously silent and callers already expected to do their own error reporting. This whole file has an unusual paradigm compared to most source files, it may be best to split this file into a separate patch (so that you aren't holding up the rest of src/util/*) and either use VIR_STRDUP_QUIET or start to tackle the bigger issue of tracing through callers to behave better when leaf functions report errors. again, double-oom no_memory label is redudant; VIR_STRDUP guarantees that 'errno == ENOMEM' if it returns -1, as do VIR_ALLOC and virAsprintf. (Hmm, maybe we should enhance './configure --enable-test-oom' to specifically test that; although it may be a surprising amount of work to get that to happen). This is a silent->noisy change, and again an instance where the cgroup callers are doing their own error reporting; and the no_memory label is a bit awkward because it is not doing its own OOM reporting. Just deleting the no_memory label, and using 'goto error' will make the code a bit less confusing. And it reiterates my thought that src/util/vircgroup.c is enough of an oddball to warrant being split into its own patch. So with that, I'll quit pointing out silent->noisy changes in this file, and just point out other problems. Yikes! Are we mixing -1 and -errno in the same function? That's an independent bug, and probably better to isolate that fix into an independent commit. double-oom. More precisely, we DON'T want to report oom until virCommandRun sees cmd->has_error==ENOMEM, so that callers have the option of building up a virCommand but ditching it halfway through if they encounter some other (more important) error. Thus, this should use VIR_STRDUP_QUIET. VIR_STRDUP_QUIET QUIET QUIET QUIET QUIET QUIET QUIET QUIET QUIET silent->noisy, caller was already doing its own reporting based on errno value. Probably a good change, but it means we probably ought to eventually touch up qemuStateInitialize to not duplicate error handling. Not for this patch, but why are we building up argv manually instead of using virCommand? This use of QUIET is correct. Simplify: if (VIR_STRDUP_QUIET(to->message, from->message) < 0) and so on. Can lose the surrounding if. and so on. silent->noisy. This function has a bug: it says it returns NULL with errno set, but then it does: if (!ret) errno = ENOENT; cleanup: endmntent(f); return ret; and endmntent is allowed to clobber errno. But I think the intent is that we DO want this function to be silent (meaning this should use QUIET), because some callers ignore certain errors. silent->noisy; looks like it needs to use _QUIET because callers ignore some errors. silent->noisy; I think some callers ignore failure, so QUIET may be better. and again. silent->noisy, but this one is probably okay. silent->noisy, but probably okay silent->noisy; probably okay, but we should adjust callers at some point silent->noisy, but probably a good change. True for most of this file. silent->noisy; there are other failure paths where we are still silent, but it's probably better to be noisy. I think you did this construct so that teh VIR_ALLOC_N also converts from silent->noisy, where a future OOM cleanup path would then switch to VIR_STRDUP instead of VIR_STRDUP_QUIET, although I'd rather live with double-oom now than forget to drop QUIET later. Hmm; this means you are silent->noisy on OOM, but still silent if name was NULL. It might be better to be consistently noisy, even if that means you can't use the <= as planned. silent->noisy, but probably good silent->noisy, but rest of function isn't consistently noisy As long as you are touching this, kill the double space. if (VIR_STRNDUP(ret->u.value, str, len > 0 ? len : strlen(str)) < 0) Might have a merge conflict with the semicolon cleanup. silent->noisy, but probably a good change. silent->noisy, but probably good and more silent->noisy; probably the whole file is impacted. Another silent->noisy, and here you were dealing with VIR_EXPAND_N. silent->noisy, but probably good. Everything else looked good. There were a few problems to fix up before pushing, and maybe it's worth resubmitting vircgroup cleanups (or doing an explicit followup), but I can live with giving: ACK I agree that we want this series in before we freeze. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

--- src/xen/block_stats.c | 16 ++++++------ src/xen/xen_driver.c | 9 +++---- src/xen/xen_hypervisor.c | 19 ++++---------- src/xen/xen_inotify.c | 15 +++-------- src/xen/xend_internal.c | 68 ++++++++++++------------------------------------ src/xen/xm_internal.c | 9 +++---- src/xen/xs_internal.c | 44 ++++++++++++------------------- 7 files changed, 56 insertions(+), 124 deletions(-) diff --git a/src/xen/block_stats.c b/src/xen/block_stats.c index ded8d7f..5952971 100644 --- a/src/xen/block_stats.c +++ b/src/xen/block_stats.c @@ -292,14 +292,14 @@ xenLinuxDomainDeviceID(int domid, const char *path) * /sys/devices/xen-backend/(vbd|tap)-{domid}-{devid}/statistics/{...} */ - if (strlen(path) >= 5 && STRPREFIX(path, "/dev/")) - mod_path = strdup(path); - else - ignore_value(virAsprintf(&mod_path, "/dev/%s", path)); - - if (!mod_path) { - virReportOOMError(); - return -1; + if (strlen(path) >= 5 && STRPREFIX(path, "/dev/")) { + if (VIR_STRDUP(mod_path, path) < 0) + return -1; + } else { + if (virAsprintf(&mod_path, "/dev/%s", path) < 0) { + virReportOOMError(); + return -1; + } } retval = -1; diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index cc54f7a..84a9fcc 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -408,10 +408,8 @@ xenUnifiedConnectOpen(virConnectPtr conn, virConnectAuthPtr auth, unsigned int f priv->opened[XEN_UNIFIED_INOTIFY_OFFSET] = 1; #endif - if (!(priv->saveDir = strdup(XEN_SAVE_DIR))) { - virReportOOMError(); + if (VIR_STRDUP(priv->saveDir, XEN_SAVE_DIR) < 0) goto error; - } if (virFileMakePath(priv->saveDir) < 0) { VIR_ERROR(_("Errored to create save dir '%s': %s"), priv->saveDir, @@ -2165,8 +2163,8 @@ xenUnifiedAddDomainInfo(xenUnifiedDomainInfoListPtr list, if (VIR_ALLOC(info) < 0) goto memory_error; - if (!(info->name = strdup(name))) - goto memory_error; + if (VIR_STRDUP(info->name, name) < 0) + goto error; memcpy(info->uuid, uuid, VIR_UUID_BUFLEN); info->id = id; @@ -2182,6 +2180,7 @@ xenUnifiedAddDomainInfo(xenUnifiedDomainInfoListPtr list, return 0; memory_error: virReportOOMError(); +error: if (info) VIR_FREE(info->name); VIR_FREE(info); diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c index 68a3ef4..590575e 100644 --- a/src/xen/xen_hypervisor.c +++ b/src/xen/xen_hypervisor.c @@ -1141,16 +1141,12 @@ xenHypervisorGetSchedulerType(virDomainPtr domain, int *nparams) switch (op.u.getschedulerid.sched_id){ case XEN_SCHEDULER_SEDF: - schedulertype = strdup("sedf"); - if (schedulertype == NULL) - virReportOOMError(); + ignore_value(VIR_STRDUP(schedulertype, "sedf")); if (nparams) *nparams = XEN_SCHED_SEDF_NPARAM; break; case XEN_SCHEDULER_CREDIT: - schedulertype = strdup("credit"); - if (schedulertype == NULL) - virReportOOMError(); + ignore_value(VIR_STRDUP(schedulertype, "credit")); if (nparams) *nparams = XEN_SCHED_CRED_NPARAM; break; @@ -2534,14 +2530,9 @@ xenHypervisorDomainGetOSType(virDomainPtr dom) return NULL; } - if (XEN_GETDOMAININFO_FLAGS(dominfo) & DOMFLAGS_HVM) - ostype = strdup("hvm"); - else - ostype = strdup("linux"); - - if (ostype == NULL) - virReportOOMError(); - + ignore_value(VIR_STRDUP(ostype, + XEN_GETDOMAININFO_FLAGS(dominfo) & DOMFLAGS_HVM ? + "hvm" : "linux")); return ostype; } diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c index e13c572..0b2c622 100644 --- a/src/xen/xen_inotify.c +++ b/src/xen/xen_inotify.c @@ -39,7 +39,7 @@ #include "virlog.h" #include "viruuid.h" #include "virfile.h" - +#include "virstring.h" #include "xm_internal.h" /* for xenXMDomainConfigParse */ #define VIR_FROM_THIS VIR_FROM_XEN_INOTIFY @@ -58,12 +58,9 @@ xenInotifyXenCacheLookup(virConnectPtr conn, return -1; } - *name = strdup(entry->def->name); memcpy(uuid, entry->def->uuid, VIR_UUID_BUFLEN); - - if (!*name) { + if (VIR_STRDUP(*name, entry->def->name) < 0) { VIR_DEBUG("Error getting dom from def"); - virReportOOMError(); return -1; } return 0; @@ -103,11 +100,8 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, list info */ for (i = 0 ; i < priv->configInfoList->count ; i++) { if (!memcmp(rawuuid, priv->configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN)) { - *name = strdup(priv->configInfoList->doms[i]->name); - if (!*name) { - virReportOOMError(); + if (VIR_STRDUP(*name, priv->configInfoList->doms[i]->name) < 0) return -1; - } memcpy(uuid, priv->configInfoList->doms[i]->uuid, VIR_UUID_BUFLEN); VIR_DEBUG("Found dom on list"); return 0; @@ -118,8 +112,7 @@ xenInotifyXendDomainsDirLookup(virConnectPtr conn, return -1; } - if (!(*name = strdup(dom->name))) { - virReportOOMError(); + if (VIR_STRDUP(*name, dom->name) < 0) { virDomainFree(dom); return -1; } diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c index aec57f5..96758cc 100644 --- a/src/xen/xend_internal.c +++ b/src/xen/xend_internal.c @@ -761,8 +761,7 @@ xenDaemonListDomainsOld(virConnectPtr xend) _for_i = _for_i->u.s.cdr, node = _for_i->u.s.car) { if (node->kind != SEXPR_VALUE) continue; - ret[i] = strdup(node->u.value); - if (!ret[i]) + if (VIR_STRDUP(ret[i], node->u.value) < 0) goto no_memory; i++; } @@ -1384,14 +1383,8 @@ xenDaemonDomainGetOSType(virDomainPtr domain) if (root == NULL) return NULL; - if (sexpr_lookup(root, "domain/image/hvm")) { - type = strdup("hvm"); - } else { - type = strdup("linux"); - } - - if (type == NULL) - virReportOOMError(); + ignore_value(VIR_STRDUP(type, + sexpr_lookup(root, "domain/image/hvm") ? "hvm" : "linux")); sexpr_free(root); @@ -2100,12 +2093,7 @@ xenDaemonLookupByUUID(virConnectPtr conn, const unsigned char *uuid) else id = -1; - if (domname) { - name = strdup(domname); - - if (name == NULL) - virReportOOMError(); - } + ignore_value(VIR_STRDUP(name, domname)); sexpr_free(root); } @@ -2260,12 +2248,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, priv->xendConfigVersion, 1) < 0) goto cleanup; - if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM) { - if (!(target = strdup(dev->data.disk->dst))) { - virReportOOMError(); - goto cleanup; - } - } + if (dev->data.disk->device != VIR_DOMAIN_DISK_DEVICE_CDROM && + VIR_STRDUP(target, dev->data.disk->dst) < 0) + goto cleanup; break; case VIR_DOMAIN_DEVICE_NET: @@ -2279,10 +2264,8 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, char macStr[VIR_MAC_STRING_BUFLEN]; virMacAddrFormat(&dev->data.net->mac, macStr); - if (!(target = strdup(macStr))) { - virReportOOMError(); + if (VIR_STRDUP(target, macStr) < 0) goto cleanup; - } break; case VIR_DOMAIN_DEVICE_HOSTDEV: @@ -2586,12 +2569,9 @@ xenDaemonDomainSetAutostart(virDomainPtr domain, int autostart) /* Change the autostart value in place, then define the new sexpr */ VIR_FREE(autonode->u.s.car->u.value); - autonode->u.s.car->u.value = (autostart ? strdup("start") - : strdup("ignore")); - if (!(autonode->u.s.car->u.value)) { - virReportOOMError(); + if (VIR_STRDUP(autonode->u.s.car->u.value, + autostart ? "start" : "ignore") < 0) goto error; - } if (sexpr2string(root, &buffer) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -2750,9 +2730,7 @@ xenDaemonDomainMigratePerform(virDomainPtr domain, virURIFree(uriptr); return -1; } - hostname = strdup(uriptr->server); - if (!hostname) { - virReportOOMError(); + if (VIR_STRDUP(hostname, uriptr->server) < 0) { virURIFree(uriptr); return -1; } @@ -2772,19 +2750,13 @@ xenDaemonDomainMigratePerform(virDomainPtr domain, /* Get the hostname. */ n = p - uri; /* n = Length of hostname in bytes. */ - hostname = strdup(uri); - if (!hostname) { - virReportOOMError(); + if (VIR_STRDUP(hostname, uri) < 0) return -1; - } hostname[n] = '\0'; } else { /* "hostname" (or IP address) */ - hostname = strdup(uri); - if (!hostname) { - virReportOOMError(); + if (VIR_STRDUP(hostname, uri) < 0) return -1; - } } VIR_DEBUG("hostname = %s, port = %s", hostname, port); @@ -2938,10 +2910,8 @@ xenDaemonListDefinedDomains(virConnectPtr conn, if (node->kind != SEXPR_VALUE) continue; - if ((names[ret++] = strdup(node->u.value)) == NULL) { - virReportOOMError(); + if (VIR_STRDUP(names[ret++], node->u.value) < 0) goto error; - } if (ret >= maxnames) break; @@ -2997,19 +2967,13 @@ xenDaemonGetSchedulerType(virDomainPtr domain, int *nparams) goto error; } if (STREQ(ret, "credit")) { - schedulertype = strdup("credit"); - if (schedulertype == NULL){ - virReportOOMError(); + if (VIR_STRDUP(schedulertype, "credit") < 0) goto error; - } if (nparams) *nparams = XEN_SCHED_CRED_NPARAM; } else if (STREQ(ret, "sedf")) { - schedulertype = strdup("sedf"); - if (schedulertype == NULL){ - virReportOOMError(); + if (VIR_STRDUP(schedulertype, "sedf") < 0) goto error; - } if (nparams) *nparams = XEN_SCHED_SEDF_NPARAM; } else { diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 66a6c4c..e64a9dd 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -241,8 +241,7 @@ xenXMConfigCacheAddFile(virConnectPtr conn, const char *filename) virReportOOMError(); return -1; } - if ((entry->filename = strdup(filename)) == NULL) { - virReportOOMError(); + if (VIR_STRDUP(entry->filename, filename) < 0) { VIR_FREE(entry); return -1; } @@ -1047,10 +1046,8 @@ xenXMDomainDefineXML(virConnectPtr conn, const char *xml) goto error; } - if ((entry->filename = strdup(filename)) == NULL) { - virReportOOMError(); + if (VIR_STRDUP(entry->filename, filename) < 0) goto error; - } entry->def = def; if (virHashAddEntry(priv->configCache, filename, entry) < 0) { @@ -1140,7 +1137,7 @@ xenXMListIterator(void *payload ATTRIBUTE_UNUSED, const void *name, void *data) dom = xenDaemonLookupByName(ctx->conn, name); if (!dom) { - if (!(ctx->names[ctx->count] = strdup(name))) + if (VIR_STRDUP(ctx->names[ctx->count], name) < 0) ctx->oom = 1; else ctx->count++; diff --git a/src/xen/xs_internal.c b/src/xen/xs_internal.c index 496c30b..eab18f1 100644 --- a/src/xen/xs_internal.c +++ b/src/xen/xs_internal.c @@ -50,6 +50,7 @@ #include "xen_driver.h" #include "xs_internal.h" #include "xen_hypervisor.h" +#include "virstring.h" #define VIR_FROM_THIS VIR_FROM_XEN @@ -428,11 +429,7 @@ xenStoreDomainGetNetworkID(virConnectPtr conn, int id, const char *mac) VIR_FREE(val); if (match) { - ret = strdup(list[i]); - - if (ret == NULL) - virReportOOMError(); - + ignore_value(VIR_STRDUP(ret, list[i])); break; } } @@ -481,10 +478,7 @@ xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) if ((devlen != len) || memcmp(val, dev, len)) { VIR_FREE(val); } else { - ret = strdup(list[i]); - - if (ret == NULL) - virReportOOMError(); + ignore_value(VIR_STRDUP(ret, list[i])); VIR_FREE(val); VIR_FREE(list); @@ -504,10 +498,7 @@ xenStoreDomainGetDiskID(virConnectPtr conn, int id, const char *dev) if ((devlen != len) || memcmp(val, dev, len)) { VIR_FREE(val); } else { - ret = strdup(list[i]); - - if (ret == NULL) - virReportOOMError(); + ignore_value(VIR_STRDUP(ret, list[i])); VIR_FREE(val); VIR_FREE(list); @@ -559,7 +550,7 @@ xenStoreDomainGetPCIID(virConnectPtr conn, int id, const char *bdf) VIR_FREE(val); if (match) { - ret = strdup(list[i]); + ignore_value(VIR_STRDUP(ret, list[i])); break; } } @@ -665,22 +656,22 @@ xenStoreAddWatch(virConnectPtr conn, } } - if (VIR_ALLOC(watch) < 0) - goto no_memory; + if (VIR_ALLOC(watch) < 0) { + virReportOOMError(); + goto error; + } - watch->path = strdup(path); - watch->token = strdup(token); - watch->cb = cb; + watch->cb = cb; watch->opaque = opaque; - - if (watch->path == NULL || watch->token == NULL) { - goto no_memory; - } + if (VIR_STRDUP(watch->path, path) < 0 || + VIR_STRDUP(watch->token, token) < 0) + goto error; /* Make space on list */ n = list->count; if (VIR_REALLOC_N(list->watches, n + 1) < 0) { - goto no_memory; + virReportOOMError(); + goto error; } list->watches[n] = watch; @@ -688,15 +679,12 @@ xenStoreAddWatch(virConnectPtr conn, return xs_watch(priv->xshandle, watch->path, watch->token); - no_memory: + error: if (watch) { VIR_FREE(watch->path); VIR_FREE(watch->token); VIR_FREE(watch); } - - virReportOOMError(); - return -1; } -- 1.8.2.1

On 23.05.2013 13:28, Ján Tomko wrote: Would it be possible to rather: diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c index 41eb928..7c05911 100644 --- a/src/xen/xm_internal.c +++ b/src/xen/xm_internal.c @@ -1171,8 +1171,6 @@ xenXMListDefinedDomains(virConnectPtr conn, char **const names, int maxnames) if (ctx.oom) { for (i = 0; i < ctx.count; i++) VIR_FREE(ctx.names[i]); - - virReportOOMError(); goto cleanup; } Michal

On 05/23/2013 09:07 AM, Michal Privoznik wrote: Yes, I like that better. ACK with that change. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 05/20/2013 07:55 PM, Michal Privoznik wrote: Change offset to offset2 at the beginning of the condition. Or you could do it with just one VIR_STRNDUP call: offset++; if (VIR_STRNDUP(def->source.data.tcp.service, offset, offset2 ? offset2 - offset : strlen(offset)) < 0) goto error; ACK Jan

On 20.05.2013 19:55, Michal Privoznik wrote: If 14/13 is accepted, then this line needs to be: ^(docs/|examples/|python/|src/util/virstring\.c$$)

On 23.05.2013 21:34, Eric Blake wrote: I think we did fix the rest of the code - at least I've not hit any issue with syntax-check. Thanks for all the fish - I mean review. Michal

On 05/23/2013 10:32 AM, Michal Privoznik wrote: Yes, we do. ACK. Wow - the 0 return value actually made sense :) -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org