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@xxxxxxxxx>
> ---
> 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
