Re: [PATCH v3 6/7] qapi: Use QAPI_LIST_APPEND in trivial cases

[Eric Blake]

On 12/24/20 3:56 AM, Vladimir Sementsov-Ogievskiy wrote:
> 24.12.2020 01:11, Eric Blake wrote:
>> The easiest spots to use QAPI_LIST_APPEND are where we already have an
>> obvious pointer to the tail of a list.  While at it, consistently use
>> the variable name 'tail' for that purpose.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
>> ---
> 
> [..]
> 
>> --- a/monitor/qmp-cmds-control.c
>> +++ b/monitor/qmp-cmds-control.c
>> @@ -104,17 +104,16 @@ VersionInfo *qmp_query_version(Error **errp)
>>
>>   static void query_commands_cb(const QmpCommand *cmd, void *opaque)
>>   {
>> -    CommandInfoList *info, **list = opaque;
>> +    CommandInfo *info;
>> +    CommandInfoList **tail = opaque;
>>
>>       if (!cmd->enabled) {
>>           return;
>>       }
>>
>>       info = g_malloc0(sizeof(*info));
>> -    info->value = g_malloc0(sizeof(*info->value));
>> -    info->value->name = g_strdup(cmd->name);
>> -    info->next = *list;
>> -    *list = info;
>> +    info->name = g_strdup(cmd->name);
>> +    QAPI_LIST_APPEND(tail, info);
> 
> Original logic is prepend in this hunk.
> 

Good catch; looks like it should be folded in with the remainder of
patch 4/7 on the respin.

> Without this hunk:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
>>   }
>>
>>   CommandInfoList *qmp_query_commands(Error **errp)
> 
> [..]
> 
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -4817,20 +4817,17 @@ static void x86_cpu_filter_features(X86CPU
>> *cpu, bool verbose);
>>
>>   /* Build a list with the name of all features on a feature word
>> array */
>>   static void x86_cpu_list_feature_names(FeatureWordArray features,
>> -                                       strList **feat_names)
>> +                                       strList **tail)
>>   {
>>       FeatureWord w;
>> -    strList **next = feat_names;
>>
>>       for (w = 0; w < FEATURE_WORDS; w++) {
>>           uint64_t filtered = features[w];
>>           int i;
>>           for (i = 0; i < 64; i++) {
>>               if (filtered & (1ULL << i)) {
>> -                strList *new = g_new0(strList, 1);
>> -                new->value = g_strdup(x86_cpu_feature_name(w, i));
>> -                *next = new;
>> -                next = &new->next;
>> +                QAPI_LIST_APPEND(tail,
>> +                                 g_strdup(x86_cpu_feature_name(w, i)));
> 
> actually, fit in one line...
> 
>>               }
>>           }
>>       }
> 
> [..]
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org