On 11/19/20 2:50 AM, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> These cases require a bit more thought to review; in each case, the
>> code was appending to a list, but not with a FOOList **tail variable.
>> +++ b/hw/core/machine-qmp-cmds.c
> [...]
>> @@ -294,41 +281,31 @@ void qmp_set_numa_node(NumaOptions *cmd, Error **errp)
>> static int query_memdev(Object *obj, void *opaque)
>> v = qobject_input_visitor_new(host_nodes);
>> - visit_type_uint16List(v, NULL, &m->value->host_nodes, &error_abort);
>> + visit_type_uint16List(v, NULL, &m->host_nodes, &error_abort);
>> visit_free(v);
>> qobject_unref(host_nodes);
>>
>> - m->next = *list;
>> - *list = m;
>> + QAPI_LIST_APPEND(list, m);
>
> The old code prepends, doesn't it?
Good catch, will correct and hoist this into 4/7 for v3.
>
>> }
>>
>> return 0;
>> diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
>> index cf0627fd01c1..1afcc29a0649 100644
>> --- a/hw/mem/memory-device.c
>> +++ b/hw/mem/memory-device.c
>> @@ -199,7 +199,7 @@ out:
>> MemoryDeviceInfoList *qmp_memory_device_list(void)
>> {
>> GSList *devices = NULL, *item;
>> - MemoryDeviceInfoList *list = NULL, *prev = NULL;
>> + MemoryDeviceInfoList *list = NULL, **prev = &list;
>
> Here, you reuse the old name for the new variable.
>> +++ b/hw/pci/pci.c
>> @@ -1681,41 +1681,34 @@ static PciDeviceInfoList
>> *qmp_query_pci_devices(PCIBus *bus, int bus_num);
>>
>> static PciMemoryRegionList *qmp_query_pci_regions(const PCIDevice *dev)
>> {
>> - PciMemoryRegionList *head = NULL, *cur_item = NULL;
>> + PciMemoryRegionList *head = NULL, **tail = &head;
>
> Here, you use a new and better name.
>
> I'd like to encourage you to name tail pointer variables @tail
> elsewhere, too.
In v3, I will consistently rename the FOOList ** variable 'tail'.
>> @@ -2863,7 +2846,6 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList
>> *mem_blks, Error **errp)
>>
>> while (mem_blks != NULL) {
>> GuestMemoryBlockResponse *result;
>> - GuestMemoryBlockResponseList *entry;
>> GuestMemoryBlock *current_mem_blk = mem_blks->value;
>>
>> result = g_malloc0(sizeof(*result));
>> @@ -2872,11 +2854,7 @@ qmp_guest_set_memory_blocks(GuestMemoryBlockList
>> *mem_blks, Error **errp)
>> if (local_err) { /* should never happen */
>> goto err;
>> }
>> - entry = g_malloc0(sizeof *entry);
>> - entry->value = result;
>> -
>> - *link = entry;
>> - link = &entry->next;
>> + QAPI_LIST_APPEND(link, result);
>> mem_blks = mem_blks->next;
>> }
>>
>
> This one looks like a candidate for PATCH 6.
Yes. Will hoist.
v3 will be posted soon.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
