[libvirt PATCH v2 0/2] Remove macros "VIR_INSERT_ELEMENT_COPY" and "VIR_INSERT_ELEMENT_COPY_INPLACE"
See https://listman.redhat.com/archives/libvir-list/2021-July/msg00163.html Changes since V1: * Fixed a mistake in the copy-out of virQEMUCapsGetMachineTypesCaps, where the array was allocated with the un-adjusted size. Tim Wiederhake (2): virQEMUCapsGetMachineTypesCaps: Use GPtrArray viralloc: Delete VIR_INSERT_ELEMENT_COPY and VIR_INSERT_ELEMENT_COPY_INPLACE src/qemu/qemu_capabilities.c | 37 ++++++++++++++++++------------------ src/util/viralloc.h | 22 ++++----------------- 2 files changed, 22 insertions(+), 37 deletions(-) -- 2.31.1
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..d9e1082d4e 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 *, array->len); + for (i = 0; i < array->len; ++i) + (*machines)[i] = g_ptr_array_index(array, i); - error: - virCapabilitiesFreeMachines(*machines, *nmachines); - *nmachines = 0; - *machines = NULL; - return -1; + return 0; } -- 2.31.1
There are no users left. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 553d2951cf..72e8f13bef 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -165,7 +165,7 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, sizeof(char[2 * (sizeof(*(a) = *(b)) == sizeof(*(b))) - 1]) /** - * VIR_INSERT_ELEMENT: + * VIR_INSERT_ELEMENT, VIR_INSERT_ELEMENT_INPLACE: * @ptr: pointer to array of objects (*not* ptr to ptr) * @at: index within array where new elements should be added * @count: variable tracking number of elements currently allocated @@ -179,18 +179,10 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, * item 'newelem' into ptr[at], then store the address of * allocated memory in 'ptr' and the new size in 'count'. * - * VIR_INSERT_ELEMENT_COPY is identical, but doesn't clear out the - * original element to 0 on success, so there are two copies of the - * element. This is useful if the "element" is actually just a - * pointer to the real data, and you want to maintain a reference to - * it for use after the insert is completed; but if the "element" is - * an object that points to other allocated memory, having multiple - * copies can cause problems (e.g. double free). + * VIR_INSERT_ELEMENT_INPLACE is identical, but assumes any necessary + * memory re-allocation has already been done. * - * VIR_INSERT_ELEMENT_*INPLACE are identical, but assume any necessary - * memory re-allocation has already been done. - * - * VIR_INSERT_ELEMENT_* all need to send "1" as the "add" argument to + * Both functions need to send "1" as the "add" argument to * virInsertElementsN (which has the currently-unused capability of * inserting multiple items at once). We use this to our advantage by * replacing it with VIR_TYPECHECK(ptr, &newelem) so that we can be @@ -203,15 +195,9 @@ int virDeleteElementsN(void *ptrptr, size_t size, size_t at, size_t *countptr, #define VIR_INSERT_ELEMENT(ptr, at, count, newelem) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, false) -#define VIR_INSERT_ELEMENT_COPY(ptr, at, count, newelem) \ - virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, false) #define VIR_INSERT_ELEMENT_INPLACE(ptr, at, count, newelem) \ virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), true, true) -#define VIR_INSERT_ELEMENT_COPY_INPLACE(ptr, at, count, newelem) \ - virInsertElementsN(&(ptr), sizeof(*(ptr)), at, &(count), \ - VIR_TYPEMATCH(ptr, &(newelem)), &(newelem), false, true) /** * VIR_APPEND_ELEMENT: -- 2.31.1
On 7/9/21 3:23 PM, Tim Wiederhake wrote:
See https://listman.redhat.com/archives/libvir-list/2021-July/msg00163.html
Changes since V1: * Fixed a mistake in the copy-out of virQEMUCapsGetMachineTypesCaps, where the array was allocated with the un-adjusted size.
Tim Wiederhake (2): virQEMUCapsGetMachineTypesCaps: Use GPtrArray viralloc: Delete VIR_INSERT_ELEMENT_COPY and VIR_INSERT_ELEMENT_COPY_INPLACE
src/qemu/qemu_capabilities.c | 37 ++++++++++++++++++------------------ src/util/viralloc.h | 22 ++++----------------- 2 files changed, 22 insertions(+), 37 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Michal Prívozník -
Tim Wiederhake