3:44 p.m.
On 05/03/2013 08:53 AM, Michal Privoznik wrote:
---
include/libvirt/libvirt.h.in | 10 +-
src/qemu/qemu_capabilities.c | 79 ++++----
src/qemu/qemu_cgroup.c | 4 +-
src/qemu/qemu_command.c | 419 +++++++++++++++++--------------------------
src/qemu/qemu_conf.c | 58 +++---
src/qemu/qemu_domain.c | 26 ++-
src/qemu/qemu_driver.c | 113 ++++--------
src/qemu/qemu_hotplug.c | 15 +-
src/qemu/qemu_migration.c | 20 +--
src/qemu/qemu_monitor_json.c | 63 ++-----
src/qemu/qemu_monitor_text.c | 15 +-
src/qemu/qemu_process.c | 64 +++----
src/remote/remote_driver.c | 2 +-
13 files changed, 333 insertions(+), 555 deletions(-)
Part 3 - the qemu changes
+++ b/src/qemu/qemu_capabilities.c
@@ -384,7 +384,8 @@ virQEMUCapsParseMachineTypesStr(const char *output,
VIR_REALLOC_N(qemuCaps->machineAliases, qemuCaps->nmachineTypes + 1)
< 0) {
VIR_FREE(name);
VIR_FREE(canonical);
Seems like we could move some of this cleanup...
- goto no_memory;
+ virReportOOMError();
+ goto error;
}
qemuCaps->nmachineTypes++;
if (canonical) {
@@ -402,8 +403,7 @@ virQEMUCapsParseMachineTypesStr(const char *output,
return 0;
-no_memory:
- virReportOOMError();
+error:
return -1;
...here, so that we don't have a 'goto/return'. But that can be a
separate patch (especially since you'll be revisiting VIR_REALLOC anyway).
@@ -1736,17 +1728,18 @@ 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 (VIR_STRDUP(ret->machineTypes[i], qemuCaps->machineTypes[i]) < 0)
+ goto error;
if (qemuCaps->machineAliases[i] &&
- !(ret->machineAliases[i] = strdup(qemuCaps->machineAliases[i])))
- goto no_memory;
+ VIR_STRDUP(ret->machineAliases[i], qemuCaps->machineAliases[i]) <
0)
+ goto error;
Can be simplified with NULL source.
@@ -1897,12 +1889,12 @@ int
virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps,
if (VIR_ALLOC(mach) < 0)
goto no_memory;
if (qemuCaps->machineAliases[i]) {
- if (!(mach->name = strdup(qemuCaps->machineAliases[i])))
+ if (VIR_STRDUP(mach->name, qemuCaps->machineAliases[i]) < 0)
goto no_memory;
double-oom, but I'm assuming you'll clean it later.
@@ -2091,16 +2083,11 @@
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 (machines[i]->alias &&
+ VIR_STRDUP(qemuCaps->machineAliases[i], machines[i]->alias) < 0)
+ goto cleanup;
Can be simplified.
+++ b/src/qemu/qemu_command.c
@@ -2418,13 +2404,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)
Pre-existing, but space around '-' while touching this.
@@ -5456,9 +5430,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 (cpu->vendor_id && VIR_STRDUP(guest->vendor_id,
cpu->vendor_id) < 0)
+ goto cleanup;
Can be simplified.
@@ -8312,17 +8287,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;
Space around '-' while touching this.
@@ -9161,14 +9119,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;
Space around '+' while touching this
@@ -9179,40 +9135,32 @@
qemuParseCommandLineChr(virDomainChrSourceDefPtr source,
host2 = svc1 ? strchr(svc1, '@') : NULL;
svc2 = host2 ? strchr(host2, ':') : NULL;
You know, we could use strchrnul() here (guaranteed by gnulib)...
- 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;
Space around '-'
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;
...then here, we could simplify things to always be a strndup (host2
would always be non-null, either because it hit ':' or the end of the
string). This whole function could be simplified by doing the string
parse with a bit more smarts.
@@ -9235,37 +9183,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;
Spaces around '-'
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;
Another place where strchrnul probably helps.
@@ -9318,13 +9254,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;
I'll quit pointing out spots where strchrnul might help - at this point
converting to use strchrnul is worth splitting into a separate cleanup
patch (whether before or after this patch, I don't know which is easier).
@@ -10189,9 +10104,8 @@ virDomainDefPtr
qemuParseCommandLine(virCapsPtr qemuCaps,
Shoot. I ran out of time for today. What I've seen so far in qemu is
looking okay, though.
+++ 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;
This one feels random (oh, it's associated with your proposed changes to
libvirt.h, so it belongs to that patch, depending on what we decide there).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org