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

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> 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>
> ---
[...]
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 6350caa76530..3f3182007b07 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -42,16 +42,12 @@ static ACPIOSTInfo *acpi_cpu_device_status(int idx, 
> AcpiCpuStatus *cdev)
>      return info;
>  }
>
> -void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
> +void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***tail)
>  {
>      int i;

Sure you want to rename the parameter?  What about:

   void acpi_cpu_ospm_status(CPUHotplugState *cpu_st, ACPIOSTInfoList ***list)
   {
  +    ACPIOSTInfoList ***tail = list;
       int i;

>
>      for (i = 0; i < cpu_st->dev_count; i++) {
> -        ACPIOSTInfoList *elem = g_new0(ACPIOSTInfoList, 1);
> -        elem->value = acpi_cpu_device_status(i, &cpu_st->devs[i]);
> -        elem->next = NULL;
> -        **list = elem;
> -        *list = &elem->next;
> +        QAPI_LIST_APPEND(*tail, acpi_cpu_device_status(i, &cpu_st->devs[i]));
>      }
>  }
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index f2552b2a4624..e4b836bd5e46 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -51,16 +51,13 @@ static ACPIOSTInfo *acpi_memory_device_status(int slot, 
> MemStatus *mdev)
>      return info;
>  }
>
> -void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList 
> ***list)
> +void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList 
> ***tail)

Likewise.

>  {
>      int i;
>
>      for (i = 0; i < mem_st->dev_count; i++) {
> -        ACPIOSTInfoList *elem = g_new0(ACPIOSTInfoList, 1);
> -        elem->value = acpi_memory_device_status(i, &mem_st->devs[i]);
> -        elem->next = NULL;
> -        **list = elem;
> -        *list = &elem->next;
> +        QAPI_LIST_APPEND(*tail,
> +                         acpi_memory_device_status(i, &mem_st->devs[i]));
>      }
>  }
>
[...]
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 35459a38bb1c..95326285b76d 100644
> --- 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)

Likewise.

>  {
>      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)));
>              }
>          }
>      }
> @@ -4851,16 +4848,13 @@ static void x86_cpu_get_unavailable_features(Object 
> *obj, Visitor *v,
>   * running using the current machine and accelerator.
>   */
>  static void x86_cpu_class_check_missing_features(X86CPUClass *xcc,
> -                                                 strList **missing_feats)
> +                                                 strList **tail)

Likewise.

>  {
>      X86CPU *xc;
>      Error *err = NULL;
> -    strList **next = missing_feats;
>
>      if (xcc->host_cpuid_required && !accel_uses_host_cpuid()) {
> -        strList *new = g_new0(strList, 1);
> -        new->value = g_strdup("kvm");
> -        *missing_feats = new;
> +        QAPI_LIST_APPEND(tail, g_strdup("kvm"));
>          return;
>      }
>
> @@ -4872,16 +4866,13 @@ static void 
> x86_cpu_class_check_missing_features(X86CPUClass *xcc,
>           * but in case it does, just report the model as not
>           * runnable at all using the "type" property.
>           */
> -        strList *new = g_new0(strList, 1);
> -        new->value = g_strdup("type");
> -        *next = new;
> -        next = &new->next;
> +        QAPI_LIST_APPEND(tail, g_strdup("type"));
>          error_free(err);
>      }
>
>      x86_cpu_filter_features(xc, false);
>
> -    x86_cpu_list_feature_names(xc->filtered_features, next);
> +    x86_cpu_list_feature_names(xc->filtered_features, tail);
>
>      object_unref(OBJECT(xc));
>  }
[...]

Other than that:
Reviewed-by: Markus Armbruster <armbru@redhat.com>