[libvirt PATCH v2 0/8] Replace VIR_{ALLOC, ALLOC_N, FREE} in src/cpu/*.
V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html Tim Wiederhake (8): cpu_ppc64: Use g_auto* in ppc64MakeCPUData cpu_map: Use g_auto* in loadData cpu_map: Use g_auto* in loadIncludes cpu_x86: Use g_auto* in virX86CpuIncompatible cpu_map: Remove unnecessary variable in loadData cpu: Replace VIR_ALLOC with g_new0 cpu: Replace VIR_ALLOC_N with g_new0 cpu: Replace VIR_FREE with g_free src/cpu/cpu.c | 6 ++--- src/cpu/cpu_map.c | 20 ++++++--------- src/cpu/cpu_ppc64.c | 59 ++++++++++++++++----------------------------- src/cpu/cpu_x86.c | 11 ++++----- 4 files changed, 36 insertions(+), 60 deletions(-) -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_ppc64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 28fbfea9ae..c0d09db696 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -399,7 +399,7 @@ static virCPUDataPtr ppc64MakeCPUData(virArch arch, virCPUppc64Data *data) { - virCPUDataPtr cpuData; + g_autoptr(virCPUData) cpuData = NULL; if (VIR_ALLOC(cpuData) < 0) return NULL; @@ -407,9 +407,9 @@ ppc64MakeCPUData(virArch arch, cpuData->arch = arch; if (ppc64DataCopy(&cpuData->data.ppc64, data) < 0) - VIR_FREE(cpuData); + return NULL; - return cpuData; + return g_steal_pointer(&cpuData); } static virCPUCompareResult -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_ppc64.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_map.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 65d244e011..53c8cbba6b 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -55,8 +55,9 @@ loadData(const char *mapfile, } for (i = 0; i < n; i++) { - char *name = virXMLPropString(nodes[i], "name"); - if (!name) { + g_autofree char *name = NULL; + + if (!(name = virXMLPropString(nodes[i], "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find %s name in CPU map '%s'"), element, mapfile); return -1; @@ -64,7 +65,6 @@ loadData(const char *mapfile, VIR_DEBUG("Load %s name %s", element, name); ctxt->node = nodes[i]; rv = callback(ctxt, name, data); - VIR_FREE(name); if (rv < 0) return -1; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_map.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_map.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 53c8cbba6b..ac98a14e2e 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -125,18 +125,16 @@ loadIncludes(xmlXPathContextPtr ctxt, return -1; for (i = 0; i < n; i++) { - char *filename = virXMLPropString(nodes[i], "filename"); - if (!filename) { + g_autofree char *filename = NULL; + + if (!(filename = virXMLPropString(nodes[i], "filename"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing 'filename' in CPU map include")); return -1; } VIR_DEBUG("Finding CPU map include '%s'", filename); - if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) { - VIR_FREE(filename); + if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) return -1; - } - VIR_FREE(filename); } return 0; -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_map.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 53c8cbba6b..ac98a14e2e 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -125,18 +125,16 @@ loadIncludes(xmlXPathContextPtr ctxt, return -1;
for (i = 0; i < n; i++) { - char *filename = virXMLPropString(nodes[i], "filename"); - if (!filename) { + g_autofree char *filename = NULL; +
You can assign to filename right away, there's no need to change the condtion.
+ if (!(filename = virXMLPropString(nodes[i], "filename"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing 'filename' in CPU map include")); return -1; } VIR_DEBUG("Finding CPU map include '%s'", filename); - if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) { - VIR_FREE(filename); + if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) return -1; - } - VIR_FREE(filename); }
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_x86.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index fdb665b01d..db1b2e55a1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -1797,7 +1797,7 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt) */ #define virX86CpuIncompatible(MSG, CPU_DEF) \ do { \ - char *flagsStr = NULL; \ + g_autofree char *flagsStr = NULL; \ if (!(flagsStr = x86FeatureNames(map, ", ", (CPU_DEF)))) { \ virReportOOMError(); \ return VIR_CPU_COMPARE_ERROR; \ @@ -1805,7 +1805,6 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt) if (message) \ *message = g_strdup_printf("%s: %s", _(MSG), flagsStr); \ VIR_DEBUG("%s: %s", MSG, flagsStr); \ - VIR_FREE(flagsStr); \ } while (0) -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_x86.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_map.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index ac98a14e2e..3fa03e707c 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -43,7 +43,6 @@ loadData(const char *mapfile, g_autofree xmlNodePtr *nodes = NULL; int n; size_t i; - int rv; if ((n = virXPathNodeSet(element, ctxt, &nodes)) < 0) return -1; @@ -64,8 +63,7 @@ loadData(const char *mapfile, } VIR_DEBUG("Load %s name %s", element, name); ctxt->node = nodes[i]; - rv = callback(ctxt, name, data); - if (rv < 0) + if (callback(ctxt, name, data) < 0) return -1; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_map.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 4 +--- src/cpu/cpu_ppc64.c | 19 +++++-------------- 2 files changed, 6 insertions(+), 17 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index 69e4205e4b..c3eef52c79 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -286,9 +286,7 @@ virCPUDataNew(virArch arch) { virCPUDataPtr data; - if (VIR_ALLOC(data) < 0) - return NULL; - + data = g_new0(virCPUData, 1); data->arch = arch; return data; diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index c0d09db696..ca2cfa0a67 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -189,9 +189,7 @@ ppc64ModelCopy(const virCPUppc64Model *model) { g_autoptr(virCPUppc64Model) copy = NULL; - if (VIR_ALLOC(copy) < 0) - return NULL; - + copy = g_new0(virCPUppc64Model, 1); copy->name = g_strdup(model->name); if (ppc64DataCopy(©->data, &model->data) < 0) @@ -283,9 +281,7 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, virCPUppc64MapPtr map = data; g_autoptr(virCPUppc64Vendor) vendor = NULL; - if (VIR_ALLOC(vendor) < 0) - return -1; - + vendor = g_new0(virCPUppc64Vendor, 1); vendor->name = g_strdup(name); if (ppc64VendorFind(map, vendor->name)) { @@ -314,9 +310,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, size_t i; int n; - if (VIR_ALLOC(model) < 0) - return -1; - + model = g_new0(virCPUppc64Model, 1); model->name = g_strdup(name); if (ppc64ModelFind(map, model->name)) { @@ -386,8 +380,7 @@ ppc64LoadMap(void) { g_autoptr(virCPUppc64Map) map = NULL; - if (VIR_ALLOC(map) < 0) - return NULL; + map = g_new0(virCPUppc64Map, 1); if (cpuMapLoad("ppc64", ppc64VendorParse, NULL, ppc64ModelParse, map) < 0) return NULL; @@ -401,9 +394,7 @@ ppc64MakeCPUData(virArch arch, { g_autoptr(virCPUData) cpuData = NULL; - if (VIR_ALLOC(cpuData) < 0) - return NULL; - + cpuData = g_new0(virCPUData, 1); cpuData->arch = arch; if (ppc64DataCopy(&cpuData->data.ppc64, data) < 0) -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 4 +--- src/cpu/cpu_ppc64.c | 19 +++++-------------- 2 files changed, 6 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Ján Tomko <jtomko@redhat.com> --- src/cpu/cpu_ppc64.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index ca2cfa0a67..6477b4bce7 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -134,9 +134,7 @@ ppc64DataCopy(virCPUppc64Data *dst, const virCPUppc64Data *src) { size_t i; - if (VIR_ALLOC_N(dst->pvr, src->len) < 0) - return -1; - + dst->pvr = g_new0(virCPUppc64PVR, src->len); dst->len = src->len; for (i = 0; i < src->len; i++) { @@ -343,9 +341,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, return -1; } - if (VIR_ALLOC_N(model->data.pvr, n) < 0) - return -1; - + model->data.pvr = g_new0(virCPUppc64PVR, n); model->data.len = n; for (i = 0; i < n; i++) { @@ -603,10 +599,7 @@ virCPUppc64GetHost(virCPUDefPtr cpu, return -1; data = &cpuData->data.ppc64; - - if (VIR_ALLOC_N(data->pvr, 1) < 0) - return -1; - + data->pvr = g_new0(virCPUppc64PVR, 1); data->len = 1; #if defined(__powerpc__) || defined(__powerpc64__) @@ -732,8 +725,7 @@ virCPUppc64DriverGetModels(char ***models) return -1; if (models) { - if (VIR_ALLOC_N(*models, map->nmodels + 1) < 0) - return -1; + *models = g_new0(char*, map->nmodels + 1); for (i = 0; i < map->nmodels; i++) (*models)[i] = g_strdup(map->models[i]->name); -- 2.26.2
Note the use of g_clear_pointer(..., g_free) in ppc64DataClear and virCPUx86Baseline. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 2 +- src/cpu/cpu_ppc64.c | 18 +++++++++--------- src/cpu/cpu_x86.c | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/cpu/cpu.c b/src/cpu/cpu.c index c3eef52c79..188c5d86b5 100644 --- a/src/cpu/cpu.c +++ b/src/cpu/cpu.c @@ -315,7 +315,7 @@ virCPUDataFree(virCPUDataPtr data) if ((driver = cpuGetSubDriver(data->arch)) && driver->dataFree) driver->dataFree(data); else - VIR_FREE(data); + g_free(data); } diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6477b4bce7..2fedcd25da 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -126,7 +126,7 @@ ppc64DataClear(virCPUppc64Data *data) if (!data) return; - VIR_FREE(data->pvr); + g_clear_pointer(&data->pvr, g_free); } static int @@ -151,8 +151,8 @@ ppc64VendorFree(virCPUppc64VendorPtr vendor) if (!vendor) return; - VIR_FREE(vendor->name); - VIR_FREE(vendor); + g_free(vendor->name); + g_free(vendor); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Vendor, ppc64VendorFree); @@ -177,8 +177,8 @@ ppc64ModelFree(virCPUppc64ModelPtr model) return; ppc64DataClear(&model->data); - VIR_FREE(model->name); - VIR_FREE(model); + g_free(model->name); + g_free(model); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Model, ppc64ModelFree); @@ -261,13 +261,13 @@ ppc64MapFree(virCPUppc64MapPtr map) for (i = 0; i < map->nmodels; i++) ppc64ModelFree(map->models[i]); - VIR_FREE(map->models); + g_free(map->models); for (i = 0; i < map->nvendors; i++) ppc64VendorFree(map->vendors[i]); - VIR_FREE(map->vendors); + g_free(map->vendors); - VIR_FREE(map); + g_free(map); } G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Map, ppc64MapFree); @@ -584,7 +584,7 @@ virCPUppc64DataFree(virCPUDataPtr data) return; ppc64DataClear(&data->data.ppc64); - VIR_FREE(data); + g_free(data); } diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index db1b2e55a1..0e533c62e1 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -493,7 +493,7 @@ virCPUx86DataFree(virCPUDataPtr data) return; virCPUx86DataClear(&data->data.x86); - VIR_FREE(data); + g_free(data); } @@ -2207,7 +2207,7 @@ x86Decode(virCPUDefPtr cpu, if (x86FeatureIsMigratable(cpuModel->features[i].name, map)) { i++; } else { - VIR_FREE(cpuModel->features[i].name); + g_free(cpuModel->features[i].name); VIR_DELETE_ELEMENT_INPLACE(cpuModel->features, i, cpuModel->nfeatures); } @@ -2892,7 +2892,7 @@ virCPUx86Baseline(virCPUDefPtr *cpus, cpu->fallback = VIR_CPU_FALLBACK_FORBID; if (!outputVendor) - VIR_FREE(cpu->vendor); + g_clear_pointer(&cpu->vendor, g_free); return g_steal_pointer(&cpu); } @@ -2914,7 +2914,7 @@ x86UpdateHostModel(virCPUDefPtr guest, return -1; if (guest->vendor_id) { - VIR_FREE(updated->vendor_id); + g_free(updated->vendor_id); updated->vendor_id = g_strdup(guest->vendor_id); } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Note the use of g_clear_pointer(..., g_free) in ppc64DataClear and virCPUx86Baseline.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 2 +- src/cpu/cpu_ppc64.c | 18 +++++++++--------- src/cpu/cpu_x86.c | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Tim Wiederhake