This simplyfies the code a bit and removes one "goto", one "VIR_FREE",
and one "VIR_INSERT_ELEMENT_COPY".
Signed-off-by: Tim Wiederhake <twiederh@redhat.com>
---
src/qemu/qemu_capabilities.c | 37 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 19 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d1cd8f11ac..04dae7d66e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -942,6 +942,7 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCaps *qemuCaps,
{
size_t i;
virQEMUCapsAccel *accel;
+ g_autoptr(GPtrArray) array = NULL;
/* Guest capabilities do not report TCG vs. KVM caps separately. We just
* take the set of machine types we probed first. */
@@ -953,13 +954,13 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCaps *qemuCaps,
*machines = NULL;
*nmachines = accel->nmachineTypes;
- if (*nmachines)
- *machines = g_new0(virCapsGuestMachine *, accel->nmachineTypes);
+ if (*nmachines == 0)
+ return 0;
+
+ array = g_ptr_array_sized_new(*nmachines);
for (i = 0; i < accel->nmachineTypes; i++) {
- virCapsGuestMachine *mach;
- mach = g_new0(virCapsGuestMachine, 1);
- (*machines)[i] = mach;
+ virCapsGuestMachine *mach = g_new0(virCapsGuestMachine, 1);
if (accel->machineTypes[i].alias) {
mach->name = g_strdup(accel->machineTypes[i].alias);
mach->canonical = g_strdup(accel->machineTypes[i].name);
@@ -968,6 +969,7 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCaps *qemuCaps,
}
mach->maxCpus = accel->machineTypes[i].maxCpus;
mach->deprecated = accel->machineTypes[i].deprecated;
+ g_ptr_array_add(array, mach);
}
/* Make sure all canonical machine types also have their own entry so that
@@ -975,18 +977,19 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCaps *qemuCaps,
* supported machine types.
*/
i = 0;
- while (i < *nmachines) {
+ while (i < array->len) {
size_t j;
bool found = false;
- virCapsGuestMachine *machine = (*machines)[i];
+ virCapsGuestMachine *machine = g_ptr_array_index(array, i);
if (!machine->canonical) {
i++;
continue;
}
- for (j = 0; j < *nmachines; j++) {
- if (STREQ(machine->canonical, (*machines)[j]->name)) {
+ for (j = 0; j < array->len; j++) {
+ virCapsGuestMachine *mach = g_ptr_array_index(array, j);
+ if (STREQ(machine->canonical, mach->name)) {
found = true;
break;
}
@@ -995,25 +998,21 @@ virQEMUCapsGetMachineTypesCaps(virQEMUCaps *qemuCaps,
if (!found) {
virCapsGuestMachine *mach;
mach = g_new0(virCapsGuestMachine, 1);
- if (VIR_INSERT_ELEMENT_COPY(*machines, i, *nmachines, mach) < 0) {
- VIR_FREE(mach);
- goto error;
- }
mach->name = g_strdup(machine->canonical);
mach->maxCpus = machine->maxCpus;
mach->deprecated = machine->deprecated;
+ g_ptr_array_insert(array, i, mach);
i++;
}
i++;
}
- return 0;
+ *nmachines = array->len;
+ *machines = g_new0(virCapsGuestMachine *, accel->nmachineTypes);
+ for (i = 0; i < array->len; ++i)
+ (*machines)[i] = g_ptr_array_remove_index(array, i);
- error:
- virCapabilitiesFreeMachines(*machines, *nmachines);
- *nmachines = 0;
- *machines = NULL;
- return -1;
+ return 0;
}
--
2.31.1
On 7/8/21 4:28 PM, Tim Wiederhake wrote: > This simplyfies the code a bit and removes one "goto", one "VIR_FREE", > and one "VIR_INSERT_ELEMENT_COPY". > > Signed-off-by: Tim Wiederhake <twiederh@redhat.com> > --- > src/qemu/qemu_capabilities.c | 37 ++++++++++++++++++------------------ > 1 file changed, 18 insertions(+), 19 deletions(-) > I'm not exactly sure what is causing this, but with this patch libvirtd crashes for me when I try to fetch capabilities: ==16567== Thread 3 rpc-worker: ==16567== Invalid read of size 8 ==16567== at 0x49CB01A: virCapabilitiesFormatGuestXML (capabilities.c:1259) ==16567== by 0x49CB6AB: virCapabilitiesFormatXML (capabilities.c:1355) ==16567== by 0xAE898B1: qemuConnectGetCapabilities (qemu_driver.c:1316) ==16567== by 0x4C47014: virConnectGetCapabilities (libvirt-host.c:467) ==16567== by 0x1328FD: remoteDispatchConnectGetCapabilities (remote_daemon_dispatch_stubs.h:766) ==16567== by 0x1328A5: remoteDispatchConnectGetCapabilitiesHelper (remote_daemon_dispatch_stubs.h:748) ==16567== by 0x4AB4C0F: virNetServerProgramDispatchCall (virnetserverprogram.c:428) ==16567== by 0x4AB478A: virNetServerProgramDispatch (virnetserverprogram.c:302) ==16567== by 0x4ABBE71: virNetServerProcessMsg (virnetserver.c:135) ==16567== by 0x4ABBF31: virNetServerHandleJob (virnetserver.c:152) ==16567== by 0x49AC6D5: virThreadPoolWorker (virthreadpool.c:159) ==16567== by 0x49ABBEB: virThreadHelper (virthread.c:241) ==16567== Address 0x8 is not stack'd, malloc'd or (recently) free'd Michal
On Fri, 2021-07-09 at 12:05 +0200, Michal Prívozník wrote: > On 7/8/21 4:28 PM, Tim Wiederhake wrote: > > This simplyfies the code a bit and removes one "goto", one > > "VIR_FREE", > > and one "VIR_INSERT_ELEMENT_COPY". > > > > Signed-off-by: Tim Wiederhake <twiederh@redhat.com> > > --- > > src/qemu/qemu_capabilities.c | 37 ++++++++++++++++++---------------- > > -- > > 1 file changed, 18 insertions(+), 19 deletions(-) > > > > I'm not exactly sure what is causing this, but with this patch libvirtd > crashes for me when I try to fetch capabilities: > > ==16567== Thread 3 rpc-worker: > ==16567== Invalid read of size 8 > ==16567== at 0x49CB01A: virCapabilitiesFormatGuestXML > (capabilities.c:1259) > ==16567== by 0x49CB6AB: virCapabilitiesFormatXML > (capabilities.c:1355) > ==16567== by 0xAE898B1: qemuConnectGetCapabilities > (qemu_driver.c:1316) > ==16567== by 0x4C47014: virConnectGetCapabilities (libvirt- > host.c:467) > ==16567== by 0x1328FD: remoteDispatchConnectGetCapabilities > (remote_daemon_dispatch_stubs.h:766) > ==16567== by 0x1328A5: remoteDispatchConnectGetCapabilitiesHelper > (remote_daemon_dispatch_stubs.h:748) > ==16567== by 0x4AB4C0F: virNetServerProgramDispatchCall > (virnetserverprogram.c:428) > ==16567== by 0x4AB478A: virNetServerProgramDispatch > (virnetserverprogram.c:302) > ==16567== by 0x4ABBE71: virNetServerProcessMsg (virnetserver.c:135) > ==16567== by 0x4ABBF31: virNetServerHandleJob (virnetserver.c:152) > ==16567== by 0x49AC6D5: virThreadPoolWorker (virthreadpool.c:159) > ==16567== by 0x49ABBEB: virThreadHelper (virthread.c:241) > ==16567== Address 0x8 is not stack'd, malloc'd or (recently) free'd > > Michal > Weird. Pipeline passed for me: https://gitlab.com/twiederh/libvirt/-/pipelines/333827544 Do you maybe have a reproducer for me, so I can investigate what's going on here? Thanks, Tim
On 7/9/21 1:05 PM, Tim Wiederhake wrote: > On Fri, 2021-07-09 at 12:05 +0200, Michal Prívozník wrote: >> On 7/8/21 4:28 PM, Tim Wiederhake wrote: >>> This simplyfies the code a bit and removes one "goto", one >>> "VIR_FREE", >>> and one "VIR_INSERT_ELEMENT_COPY". >>> >>> Signed-off-by: Tim Wiederhake <twiederh@redhat.com> >>> --- >>> src/qemu/qemu_capabilities.c | 37 ++++++++++++++++++---------------- >>> -- >>> 1 file changed, 18 insertions(+), 19 deletions(-) >>> >> >> I'm not exactly sure what is causing this, but with this patch libvirtd >> crashes for me when I try to fetch capabilities: >> >> ==16567== Thread 3 rpc-worker: >> ==16567== Invalid read of size 8 >> ==16567== at 0x49CB01A: virCapabilitiesFormatGuestXML >> (capabilities.c:1259) >> ==16567== by 0x49CB6AB: virCapabilitiesFormatXML >> (capabilities.c:1355) >> ==16567== by 0xAE898B1: qemuConnectGetCapabilities >> (qemu_driver.c:1316) >> ==16567== by 0x4C47014: virConnectGetCapabilities (libvirt- >> host.c:467) >> ==16567== by 0x1328FD: remoteDispatchConnectGetCapabilities >> (remote_daemon_dispatch_stubs.h:766) >> ==16567== by 0x1328A5: remoteDispatchConnectGetCapabilitiesHelper >> (remote_daemon_dispatch_stubs.h:748) >> ==16567== by 0x4AB4C0F: virNetServerProgramDispatchCall >> (virnetserverprogram.c:428) >> ==16567== by 0x4AB478A: virNetServerProgramDispatch >> (virnetserverprogram.c:302) >> ==16567== by 0x4ABBE71: virNetServerProcessMsg (virnetserver.c:135) >> ==16567== by 0x4ABBF31: virNetServerHandleJob (virnetserver.c:152) >> ==16567== by 0x49AC6D5: virThreadPoolWorker (virthreadpool.c:159) >> ==16567== by 0x49ABBEB: virThreadHelper (virthread.c:241) >> ==16567== Address 0x8 is not stack'd, malloc'd or (recently) free'd >> >> Michal >> > > Weird. Pipeline passed for me: > https://gitlab.com/twiederh/libvirt/-/pipelines/333827544 > > Do you maybe have a reproducer for me, so I can investigate what's > going on here? All I did was 'virsh capabilities'. I don't think that's something that our CI tests because we mostly construct capabilities structure from scratch. Michal
© 2016 - 2026 Red Hat, Inc.