Our internal APIs mostly use virArch rather than strings. Switching cpuGetModels to virArch will save us from unnecessary conversions in the future. Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu.c | 17 ++++------------- src/cpu/cpu.h | 3 +-- src/driver-hypervisor.h | 2 +- src/qemu/qemu_driver.c | 11 ++++++++++- src/test/test_driver.c | 12 +++++++++++- 5 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 1952b53..69055e2 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -715,7 +715,7 @@ cpuModelIsAllowed(const char *model, /** * cpuGetModels: * - * @archName: CPU architecture string + * @arch: CPU architecture * @models: where to store the list of supported models * * Fetches all CPU models supported by libvirt on @archName. @@ -723,26 +723,17 @@ cpuModelIsAllowed(const char *model, * Returns number of supported CPU models or -1 on error. */ int -cpuGetModels(const char *archName, char ***models) +cpuGetModels(virArch arch, char ***models) { struct cpuArchDriver *driver; - virArch arch; - VIR_DEBUG("arch=%s", archName); - - arch = virArchFromString(archName); - if (arch == VIR_ARCH_NONE) { - virReportError(VIR_ERR_INVALID_ARG, - _("cannot find architecture %s"), - archName); - return -1; - } + VIR_DEBUG("arch=%s", virArchToString(arch)); driver = cpuGetSubDriver(arch); if (driver == NULL) { virReportError(VIR_ERR_INVALID_ARG, _("cannot find a driver for the architecture %s"), - archName); + virArchToString(arch)); return -1; } diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h index f15dc16..5192607 100644 --- a/src/cpu/cpu.h +++ b/src/cpu/cpu.h @@ -199,8 +199,7 @@ cpuModelIsAllowed(const char *model, ATTRIBUTE_NONNULL(1); int -cpuGetModels(const char *arch, char ***models) - ATTRIBUTE_NONNULL(1); +cpuGetModels(virArch arch, char ***models); /* cpuDataFormat and cpuDataParse are implemented for unit tests only and * have no real-life usage diff --git a/src/driver-hypervisor.h b/src/driver-hypervisor.h index d11ff7f..89ff446 100644 --- a/src/driver-hypervisor.h +++ b/src/driver-hypervisor.h @@ -677,7 +677,7 @@ typedef char * typedef int (*virDrvConnectGetCPUModelNames)(virConnectPtr conn, - const char *args, + const char *archName, char ***models, unsigned int flags); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c4c4968..171fddb 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18647,14 +18647,23 @@ qemuNodeSuspendForDuration(virConnectPtr conn, static int qemuConnectGetCPUModelNames(virConnectPtr conn, - const char *arch, + const char *archName, char ***models, unsigned int flags) { + virArch arch; + virCheckFlags(0, -1); if (virConnectGetCPUModelNamesEnsureACL(conn) < 0) return -1; + if (!(arch = virArchFromString(archName))) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find architecture %s"), + archName); + return -1; + } + return cpuGetModels(arch, models); } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index a51eb09..aba98ca 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5696,11 +5696,21 @@ testDomainScreenshot(virDomainPtr dom ATTRIBUTE_UNUSED, static int testConnectGetCPUModelNames(virConnectPtr conn ATTRIBUTE_UNUSED, - const char *arch, + const char *archName, char ***models, unsigned int flags) { + virArch arch; + virCheckFlags(0, -1); + + if (!(arch = virArchFromString(archName))) { + virReportError(VIR_ERR_INVALID_ARG, + _("cannot find architecture %s"), + archName); + return -1; + } + return cpuGetModels(arch, models); } -- 2.8.2

Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com&gt; --- src/cpu/cpu_ppc64.c | 4 +++- src/cpu/cpu_x86.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 364c8ed..1aed984 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -886,8 +886,10 @@ ppc64DriverGetModels(char ***models) if (VIR_STRDUP(name, model->name) < 0) goto error; - if (VIR_APPEND_ELEMENT(*models, nmodels, name) < 0) + if (VIR_APPEND_ELEMENT(*models, nmodels, name) < 0) { + VIR_FREE(name); goto error; + } } else { nmodels++; } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index b7f1690..c6aacc1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2219,8 +2219,10 @@ x86GetModels(char ***models) if (VIR_STRDUP(name, model->name) < 0) goto error; - if (VIR_APPEND_ELEMENT(*models, nmodels, name) < 0) + if (VIR_APPEND_ELEMENT(*models, nmodels, name) < 0) { + VIR_FREE(name); goto error; + } } else { nmodels++; } -- 2.8.2

On Fri, May 13, 2016 at 11:23:30PM +0200, Jiri Denemark wrote: This fixes the function only in case that there is no failure. If the VIR_EXPAND_N() or VIR_APPEND_ELEMENT() fails we will hit segfault in virStringFreeList(). We need to preallocate the whole array with nmodels + 1. Pavel