Re: [PATCH V3 3/5] cpu: Introduce ARM related structs
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(a)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