[PATCH V3 3/5] cpu: Introduce ARM related structs

[Jiri Denemark]

On Wed, Apr 22, 2020 at 15:11:20 +0800, ZhengZhenyu wrote:
> Introduce vendor and model struct and related
> cleanup functions for ARM cpu.
> 
> Signed-off-by: Zhenyu Zheng <zhengzhenyulixi at gmail.com>
> ---
>  src/cpu/cpu_arm.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 73 insertions(+)
> 
> diff --git a/src/cpu/cpu_arm.c b/src/cpu/cpu_arm.c
> index ee5802198f..2009904cc9 100644
> --- a/src/cpu/cpu_arm.c
> +++ b/src/cpu/cpu_arm.c
...
> @@ -81,12 +103,62 @@ virCPUarmMapNew(void)
>      return map;
>  }
>  
> +static void
> +virCPUarmDataClear(virCPUarmData *data)
> +{
> +    if (!data)
> +        return;
> +
> +    VIR_FREE(data->features);

virStringListFree(data->features)

> +}
> +
> +static void
> +virCPUarmDataFree(virCPUDataPtr cpuData)
> +{
> +    if (!cpuData)
> +        return;
> +
> +    virCPUarmDataClear(&cpuData->data.arm);
> +    VIR_FREE(cpuData);

g_free()

> +}

The two functions above should go in one patch with the structure
definition. Either you can move them to the previous patch or you can
squash the two patches into a single one.

> +
> +static void
> +virCPUarmModelFree(virCPUarmModelPtr model)
> +{
> +    if (!model)
> +        return;
> +
> +    virCPUarmDataClear(&model->data);
> +    g_free(model->name);
> +    g_free(model);
> +}

Please, define the autoptr clean function for both model and vendor
structures so that you can later use g_autoptr(virCPUarm...) ... = NULL;
declarations:

    G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUarmModel, virCPUarmModelFree);

> +
> +static void
> +virCPUarmVendorFree(virCPUarmVendorPtr vendor)
> +{
> +    if (!vendor)
> +        return;
> +
> +    g_free(vendor->name);
> +    g_free(vendor);
> +}

    G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUarmVendor, virCPUarmVendorFree);

> +
>  static void
>  virCPUarmMapFree(virCPUarmMapPtr map)
>  {
>      if (!map)
>          return;
>  
> +    size_t i;

We declare all variables in the beginning of each block, i.e., this
should go above the if (!map) check.

> +
> +    for (i = 0; i < map->nmodels; i++)
> +        virCPUarmModelFree(map->models[i]);
> +    g_free(map->models);
> +
> +    for (i = 0; i < map->nvendors; i++)
> +        virCPUarmVendorFree(map->vendors[i]);
> +    g_free(map->vendors);
> +
>      g_ptr_array_free(map->features, TRUE);
>  
>      g_free(map);
> @@ -259,6 +331,7 @@ struct cpuArchDriver cpuDriverArm = {
>      .compare = virCPUarmCompare,
>      .decode = NULL,
>      .encode = NULL,
> +    .dataFree = virCPUarmDataFree,
>      .baseline = virCPUarmBaseline,
>      .update = virCPUarmUpdate,
>      .validateFeatures = virCPUarmValidateFeatures,

And the same applies for this hunk: it should go into the patch which
introduced virCPUarmData.

Jirka