On 1/4/24 01:16, Artem Chernyshev wrote:
virCPUx86DataAddItem() return value is invariant, so change it type and remove all dependent checks.
Functions changed to void:
virCPUx86DataAddItem()
x86DataAdd() virCPUx86DataAdd() x86DataAddSignature() virCPUx86DataSetSignature() libxlCapsAddCPUID() cpuidSetLeaf4() cpuidSetLeaf7() cpuidSetLeafB() cpuidSetLeafD() cpuidSetLeafResID() cpuidSetLeaf12() cpuidSetLeaf14() cpuidSetLeaf17()
In virCPUx86GetHost() still exists this statement
``` if (cpuidSet(CPUX86_BASIC, cpuData) < 0 || cpuidSet(CPUX86_EXTENDED, cpuData) < 0) return -1; ```
Probably should change it as well.
Fixes: 592517636f ("util: alloc: Reimplement VIR_APPEND_ELEMENT_COPY using virAppendElement")
It doesn't really fix anything. The code is not broken.
Signed-off-by: Artem Chernyshev <artem.chernyshev@red-soft.ru> --- src/cpu/cpu_x86.c | 179 +++++++++++++-------------------- src/cpu/cpu_x86.h | 4 +- src/libxl/libxl_capabilities.c | 13 +-- src/qemu/qemu_capabilities.c | 3 +- src/qemu/qemu_monitor_json.c | 7 +- 5 files changed, 77 insertions(+), 129 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 1574723624..622e977298 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6701,10 +6701,9 @@ qemuMonitorJSONParseCPUx86Features(virJSONValue *data)
item.type = VIR_CPU_X86_DATA_CPUID; for (i = 0; i < virJSONValueArraySize(data); i++) { - if (qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i), - &item.data.cpuid) < 0 || - virCPUx86DataAdd(cpudata, &item) < 0) - return NULL; + if (!qemuMonitorJSONParseCPUx86FeatureWord(virJSONValueArrayGet(data, i), + &item.data.cpuid)) + virCPUx86DataAdd(cpudata, &item);
No. This changes semantics. Prior to your change, if qemuMonitorJSONParseCPUx86FeatureWord() failed then 'return NULL' path would be taken. After your change the failure is ignored and ...
}
return g_steal_pointer(&cpudata);
... this return path is taken. Michal