Re: [libvirt] [PATCH v2 14/33] cpu_x86: Drop virCPUx86MakeData and use virCPUDataNew
On Tue, Feb 21, 2017 at 14:42:39 -0500, John Ferlan wrote:
...
> @@ -131,20 +130,23 @@ libxlCapsNodeData(virCPUDefPtr cpu,
libxl_hwcap hwcap,
> { .eax_in = 0x80000007, .ecx_in = 0U, .edx = hwcap[7] },
> };
>
> + if (!(cpudata = virCPUDataNew(cpu->arch)))
> + goto error;
> +
> ncaps = ARRAY_CARDINALITY(cpuid);
> - if (libxlCapsAddCPUID(&data, cpuid, ncaps) < 0)
> + if (libxlCapsAddCPUID(cpudata, cpuid, ncaps) < 0)
> goto error;
>
> ncaps = ARRAY_CARDINALITY(cpuid_ver1);
> if (version > LIBXL_HWCAP_V0 &&
> - libxlCapsAddCPUID(&data, cpuid_ver1, ncaps) < 0)
> + libxlCapsAddCPUID(cpudata, cpuid_ver1, ncaps) < 0)
> goto error;
>
> - cpudata = virCPUx86MakeData(cpu->arch, &data);
> + return cpudata;
>
> error:
> - virCPUx86DataClear(&data);
> - return cpudata;
> + cpuDataFree(cpudata);
Why is this not x86FreeCPUData ? Or should other code use the
cpuDataFree? Am I missing something subtle?
You are. Previously we were freeing virCPUx86Data data, while now the
code is freeing virCPUDataPtr cpudata. Originally cpudata was created
from data at the very end of the successful path in this function, but
now we have cpudata all time.
> diff --git a/src/qemu/qemu_monitor_json.c
b/src/qemu/qemu_monitor_json.c
> index 415761525..b4da12167 100644
> --- a/src/qemu/qemu_monitor_json.c
> +++ b/src/qemu/qemu_monitor_json.c
> @@ -6565,37 +6565,35 @@ qemuMonitorJSONParseCPUx86FeatureWord(virJSONValuePtr data,
> }
>
>
> -static int
> -qemuMonitorJSONParseCPUx86Features(virJSONValuePtr data,
> - virCPUDataPtr *cpudata)
> +static virCPUDataPtr
> +qemuMonitorJSONParseCPUx86Features(virJSONValuePtr data)
> {
> - virCPUx86Data x86Data = VIR_CPU_X86_DATA_INIT;
> + virCPUDataPtr cpudata = NULL;
> virCPUx86CPUID cpuid;
> size_t i;
> ssize_t n;
> - int ret = -1;
>
> if (!data || (n = virJSONValueArraySize(data)) < 0) {
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("invalid array of CPUID features"));
> - return -1;
> + goto error;
Could just return NULL, right?
Yes.
> }
>
> + if (!(cpudata = virCPUDataNew(VIR_ARCH_X86_64)))
> + goto error;
> +
> for (i = 0; i < n; i++) {
> if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i),
> &cpuid) < 0 ||
> - virCPUx86DataAddCPUID(&x86Data, &cpuid) < 0)
> - goto cleanup;
> + virCPUx86DataAddCPUID(&cpudata->data.x86, &cpuid) < 0)
> + goto error;
> }
>
> - if (!(*cpudata = virCPUx86MakeData(VIR_ARCH_X86_64, &x86Data)))
> - goto cleanup;
> + return cpudata;
>
> - ret = 0;
> -
> - cleanup:
> - virCPUx86DataClear(&x86Data);
> - return ret;
> + error:
> + cpuDataFree(cpudata);
Similar query about x86FreeCPUData usage.
Similar answer.
Jirka