[PATCH 00/12] assorted cleanups in cpu_ppc64.c
Hi, While reading and studying cpu_ppc64.c I noticed that it lacks the 'sophistication' that cpu_x86.c and cpu_arm.c enjoys. This is my attempt to tidy it up a bit, adding some typedefs and g_auto* cleanups to make it more up to par with cpu_x86.c Daniel Henrique Barboza (12): cpu_ppc64.c: use typedefs for 'struct ppc64_vendor' cpu_ppc64.c: register AUTOPTR_CLEANUP_FUNC for virCPUppc64VendorPtr cpu_ppc64.c: modernize ppc64VendorParse() cpu_ppc64.c: use typedefs for 'struct ppc64_model' cpu_ppc64.c: register AUTOPTR_CLEANUP_FUNC for virCPUppc64ModelPtr cpu_ppc64.c: use g_autopr() with virCPUppc64ModelPtr cpu_ppc64.c: use typedefs for 'struct ppc64_map' cpu_ppc64.c: register AUTOPTR_CLEANUP_FUNC for virCPUppc64MapPtr cpu_ppc64.c: use g_autopr() with virCPUppc64MapPtr cpu_ppc64.c: use g_autoptr() in virCPUppc64GetHost() cpu_ppc64.c: use g_autofree() whenever possible cpu_ppc64.c: use g_autoptr() whenever possible src/cpu/cpu_ppc64.c | 245 ++++++++++++++++++-------------------------- 1 file changed, 98 insertions(+), 147 deletions(-) -- 2.26.2
Introduce virCPUppc64Vendor and virCPUppc64VendorPtr types to improve code readability. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 5b34c00a18..2a7c955781 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -34,19 +34,21 @@ VIR_LOG_INIT("cpu.cpu_ppc64"); static const virArch archs[] = { VIR_ARCH_PPC64, VIR_ARCH_PPC64LE }; -struct ppc64_vendor { +typedef struct _ppc64_vendor virCPUppc64Vendor; +typedef struct _ppc64_vendor *virCPUppc64VendorPtr; +struct _ppc64_vendor { char *name; }; struct ppc64_model { char *name; - const struct ppc64_vendor *vendor; + const virCPUppc64Vendor *vendor; virCPUppc64Data data; }; struct ppc64_map { size_t nvendors; - struct ppc64_vendor **vendors; + virCPUppc64VendorPtr *vendors; size_t nmodels; struct ppc64_model **models; }; @@ -142,7 +144,7 @@ ppc64DataCopy(virCPUppc64Data *dst, const virCPUppc64Data *src) } static void -ppc64VendorFree(struct ppc64_vendor *vendor) +ppc64VendorFree(virCPUppc64VendorPtr vendor) { if (!vendor) return; @@ -151,7 +153,7 @@ ppc64VendorFree(struct ppc64_vendor *vendor) VIR_FREE(vendor); } -static struct ppc64_vendor * +static virCPUppc64VendorPtr ppc64VendorFind(const struct ppc64_map *map, const char *name) { @@ -276,7 +278,7 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, void *data) { struct ppc64_map *map = data; - struct ppc64_vendor *vendor; + virCPUppc64VendorPtr vendor; int ret = -1; if (VIR_ALLOC(vendor) < 0) @@ -691,7 +693,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, { struct ppc64_map *map; const struct ppc64_model *model; - const struct ppc64_vendor *vendor = NULL; + const virCPUppc64Vendor *vendor = NULL; virCPUDefPtr cpu = NULL; size_t i; @@ -705,7 +707,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, } for (i = 0; i < ncpus; i++) { - const struct ppc64_vendor *vnd; + const virCPUppc64Vendor *vnd; /* Hosts running old (<= 1.2.18) versions of libvirt will report * strings like 'power7+' or 'power8e' instead of proper CPU model -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Introduce virCPUppc64Vendor and virCPUppc64VendorPtr types to improve code readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Next patch will use g_autoptr() in virCPUppc64VendorPtr pointers for some cleanups. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 2a7c955781..b003e48bed 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -152,6 +152,7 @@ ppc64VendorFree(virCPUppc64VendorPtr vendor) VIR_FREE(vendor->name); VIR_FREE(vendor); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Vendor, ppc64VendorFree); static virCPUppc64VendorPtr ppc64VendorFind(const struct ppc64_map *map, -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Next patch will use g_autoptr() in virCPUppc64VendorPtr pointers for some cleanups.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use g_autoptr() in virCPUppc64VendorPtr and remove the now uneeded 'cleanup' label. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index b003e48bed..6c73145f06 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -279,8 +279,7 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, void *data) { struct ppc64_map *map = data; - virCPUppc64VendorPtr vendor; - int ret = -1; + g_autoptr(virCPUppc64Vendor) vendor = NULL; if (VIR_ALLOC(vendor) < 0) return -1; @@ -290,17 +289,13 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, if (ppc64VendorFind(map, vendor->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU vendor %s already defined"), vendor->name); - goto cleanup; + return -1; } if (VIR_APPEND_ELEMENT(map->vendors, map->nvendors, vendor) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - ppc64VendorFree(vendor); - return ret; + return 0; } -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Use g_autoptr() in virCPUppc64VendorPtr and remove the now uneeded 'cleanup' label.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Introduce virCPUppc64Model and virCPUppc64ModelPtr types to improve code readability. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6c73145f06..7113c8ac33 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -40,7 +40,9 @@ struct _ppc64_vendor { char *name; }; -struct ppc64_model { +typedef struct _ppc64_model virCPUppc64Model; +typedef struct _ppc64_model *virCPUppc64ModelPtr; +struct _ppc64_model { char *name; const virCPUppc64Vendor *vendor; virCPUppc64Data data; @@ -50,7 +52,7 @@ struct ppc64_map { size_t nvendors; virCPUppc64VendorPtr *vendors; size_t nmodels; - struct ppc64_model **models; + virCPUppc64ModelPtr *models; }; /* Convert a legacy CPU definition by transforming @@ -169,7 +171,7 @@ ppc64VendorFind(const struct ppc64_map *map, } static void -ppc64ModelFree(struct ppc64_model *model) +ppc64ModelFree(virCPUppc64ModelPtr model) { if (!model) return; @@ -179,10 +181,10 @@ ppc64ModelFree(struct ppc64_model *model) VIR_FREE(model); } -static struct ppc64_model * -ppc64ModelCopy(const struct ppc64_model *model) +static virCPUppc64ModelPtr +ppc64ModelCopy(const virCPUppc64Model *model) { - struct ppc64_model *copy; + virCPUppc64ModelPtr copy; if (VIR_ALLOC(copy) < 0) goto error; @@ -201,7 +203,7 @@ ppc64ModelCopy(const struct ppc64_model *model) return NULL; } -static struct ppc64_model * +static virCPUppc64ModelPtr ppc64ModelFind(const struct ppc64_map *map, const char *name) { @@ -215,7 +217,7 @@ ppc64ModelFind(const struct ppc64_map *map, return NULL; } -static struct ppc64_model * +static virCPUppc64ModelPtr ppc64ModelFindPVR(const struct ppc64_map *map, uint32_t pvr) { @@ -223,7 +225,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map, size_t j; for (i = 0; i < map->nmodels; i++) { - struct ppc64_model *model = map->models[i]; + virCPUppc64ModelPtr model = map->models[i]; for (j = 0; j < model->data.len; j++) { if ((pvr & model->data.pvr[j].mask) == model->data.pvr[j].value) return model; @@ -233,11 +235,11 @@ ppc64ModelFindPVR(const struct ppc64_map *map, return NULL; } -static struct ppc64_model * +static virCPUppc64ModelPtr ppc64ModelFromCPU(const virCPUDef *cpu, const struct ppc64_map *map) { - struct ppc64_model *model; + virCPUppc64ModelPtr model; if (!cpu->model) { virReportError(VIR_ERR_INVALID_ARG, "%s", @@ -305,7 +307,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, void *data) { struct ppc64_map *map = data; - struct ppc64_model *model; + virCPUppc64ModelPtr model; xmlNodePtr *nodes = NULL; char *vendor = NULL; unsigned long pvr; @@ -428,8 +430,8 @@ ppc64Compute(virCPUDefPtr host, char **message) { struct ppc64_map *map = NULL; - struct ppc64_model *host_model = NULL; - struct ppc64_model *guest_model = NULL; + virCPUppc64ModelPtr host_model = NULL; + virCPUppc64ModelPtr guest_model = NULL; virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; virArch arch; @@ -588,7 +590,7 @@ ppc64DriverDecode(virCPUDefPtr cpu, { int ret = -1; struct ppc64_map *map; - const struct ppc64_model *model; + const virCPUppc64Model *model; if (!data || !(map = ppc64LoadMap())) return -1; @@ -688,7 +690,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, bool migratable G_GNUC_UNUSED) { struct ppc64_map *map; - const struct ppc64_model *model; + const virCPUppc64Model *model; const virCPUppc64Vendor *vendor = NULL; virCPUDefPtr cpu = NULL; size_t i; -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Introduce virCPUppc64Model and virCPUppc64ModelPtr types to improve code readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Next patch will use g_autoptr() in virCPUppc64ModelPtr pointers for some cleanups. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 7113c8ac33..02ee4b0ffd 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -180,6 +180,7 @@ ppc64ModelFree(virCPUppc64ModelPtr model) VIR_FREE(model->name); VIR_FREE(model); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Model, ppc64ModelFree); static virCPUppc64ModelPtr ppc64ModelCopy(const virCPUppc64Model *model) -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Next patch will use g_autoptr() in virCPUppc64ModelPtr pointers for some cleanups.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use autocleanup with virCPUppc64ModelPtr to simplify existing code. Remove the 'error' label in ppc64ModelCopy() since it is now obsolete. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 02ee4b0ffd..cae96c94a1 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -185,23 +185,19 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Model, ppc64ModelFree); static virCPUppc64ModelPtr ppc64ModelCopy(const virCPUppc64Model *model) { - virCPUppc64ModelPtr copy; + g_autoptr(virCPUppc64Model) copy = NULL; if (VIR_ALLOC(copy) < 0) - goto error; + return NULL; copy->name = g_strdup(model->name); if (ppc64DataCopy(©->data, &model->data) < 0) - goto error; + return NULL; copy->vendor = model->vendor; - return copy; - - error: - ppc64ModelFree(copy); - return NULL; + return g_steal_pointer(©); } static virCPUppc64ModelPtr @@ -308,7 +304,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, void *data) { struct ppc64_map *map = data; - virCPUppc64ModelPtr model; + g_autoptr(virCPUppc64Model) model = NULL; xmlNodePtr *nodes = NULL; char *vendor = NULL; unsigned long pvr; @@ -382,7 +378,6 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, ret = 0; cleanup: - ppc64ModelFree(model); VIR_FREE(vendor); VIR_FREE(nodes); return ret; @@ -431,8 +426,8 @@ ppc64Compute(virCPUDefPtr host, char **message) { struct ppc64_map *map = NULL; - virCPUppc64ModelPtr host_model = NULL; - virCPUppc64ModelPtr guest_model = NULL; + g_autoptr(virCPUppc64Model) host_model = NULL; + g_autoptr(virCPUppc64Model) guest_model = NULL; virCPUDefPtr cpu = NULL; virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; virArch arch; @@ -545,8 +540,6 @@ ppc64Compute(virCPUDefPtr host, cleanup: virCPUDefFree(cpu); ppc64MapFree(map); - ppc64ModelFree(host_model); - ppc64ModelFree(guest_model); return ret; } -- 2.26.2
s/autopr/autoptr/ On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Use autocleanup with virCPUppc64ModelPtr to simplify existing code. Remove the 'error' label in ppc64ModelCopy() since it is now obsolete.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Introduce virCPUppc64Map and virCPUppc64MapPtr types to improve code readability. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index cae96c94a1..261fe3dbe6 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -48,7 +48,9 @@ struct _ppc64_model { virCPUppc64Data data; }; -struct ppc64_map { +typedef struct _ppc64_map virCPUppc64Map; +typedef struct _ppc64_map *virCPUppc64MapPtr; +struct _ppc64_map { size_t nvendors; virCPUppc64VendorPtr *vendors; size_t nmodels; @@ -157,7 +159,7 @@ ppc64VendorFree(virCPUppc64VendorPtr vendor) G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Vendor, ppc64VendorFree); static virCPUppc64VendorPtr -ppc64VendorFind(const struct ppc64_map *map, +ppc64VendorFind(const virCPUppc64Map *map, const char *name) { size_t i; @@ -201,7 +203,7 @@ ppc64ModelCopy(const virCPUppc64Model *model) } static virCPUppc64ModelPtr -ppc64ModelFind(const struct ppc64_map *map, +ppc64ModelFind(const virCPUppc64Map *map, const char *name) { size_t i; @@ -215,7 +217,7 @@ ppc64ModelFind(const struct ppc64_map *map, } static virCPUppc64ModelPtr -ppc64ModelFindPVR(const struct ppc64_map *map, +ppc64ModelFindPVR(const virCPUppc64Map *map, uint32_t pvr) { size_t i; @@ -234,7 +236,7 @@ ppc64ModelFindPVR(const struct ppc64_map *map, static virCPUppc64ModelPtr ppc64ModelFromCPU(const virCPUDef *cpu, - const struct ppc64_map *map) + const virCPUppc64Map *map) { virCPUppc64ModelPtr model; @@ -254,7 +256,7 @@ ppc64ModelFromCPU(const virCPUDef *cpu, } static void -ppc64MapFree(struct ppc64_map *map) +ppc64MapFree(virCPUppc64MapPtr map) { size_t i; @@ -277,7 +279,7 @@ ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, const char *name, void *data) { - struct ppc64_map *map = data; + virCPUppc64MapPtr map = data; g_autoptr(virCPUppc64Vendor) vendor = NULL; if (VIR_ALLOC(vendor) < 0) @@ -303,7 +305,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, const char *name, void *data) { - struct ppc64_map *map = data; + virCPUppc64MapPtr map = data; g_autoptr(virCPUppc64Model) model = NULL; xmlNodePtr *nodes = NULL; char *vendor = NULL; @@ -384,10 +386,10 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, } -static struct ppc64_map * +static virCPUppc64MapPtr ppc64LoadMap(void) { - struct ppc64_map *map; + virCPUppc64MapPtr map; if (VIR_ALLOC(map) < 0) goto error; @@ -425,7 +427,7 @@ ppc64Compute(virCPUDefPtr host, virCPUDataPtr *guestData, char **message) { - struct ppc64_map *map = NULL; + virCPUppc64MapPtr map = NULL; g_autoptr(virCPUppc64Model) host_model = NULL; g_autoptr(virCPUppc64Model) guest_model = NULL; virCPUDefPtr cpu = NULL; @@ -583,7 +585,7 @@ ppc64DriverDecode(virCPUDefPtr cpu, virDomainCapsCPUModelsPtr models) { int ret = -1; - struct ppc64_map *map; + virCPUppc64MapPtr map; const virCPUppc64Model *model; if (!data || !(map = ppc64LoadMap())) @@ -683,7 +685,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, const char **features G_GNUC_UNUSED, bool migratable G_GNUC_UNUSED) { - struct ppc64_map *map; + virCPUppc64MapPtr map; const virCPUppc64Model *model; const virCPUppc64Vendor *vendor = NULL; virCPUDefPtr cpu = NULL; @@ -772,7 +774,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, static int virCPUppc64DriverGetModels(char ***models) { - struct ppc64_map *map; + virCPUppc64MapPtr map; size_t i; int ret = -1; -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Introduce virCPUppc64Map and virCPUppc64MapPtr types to improve code readability.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Next patch will use g_autoptr() in virCPUppc64MapPtr pointers for some cleanups. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 261fe3dbe6..607b5bc322 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -273,6 +273,7 @@ ppc64MapFree(virCPUppc64MapPtr map) VIR_FREE(map); } +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCPUppc64Map, ppc64MapFree); static int ppc64VendorParse(xmlXPathContextPtr ctxt G_GNUC_UNUSED, -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Next patch will use g_autoptr() in virCPUppc64MapPtr pointers for some cleanups.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 1 + 1 file changed, 1 insertion(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use autocleanup with virCPUppc64MapPtr to simplify existing code. Remove labels when possible. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 47 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 33 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 607b5bc322..b7c4864200 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -390,19 +390,15 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, static virCPUppc64MapPtr ppc64LoadMap(void) { - virCPUppc64MapPtr map; + g_autoptr(virCPUppc64Map) map = NULL; if (VIR_ALLOC(map) < 0) - goto error; + return NULL; if (cpuMapLoad("ppc64", ppc64VendorParse, NULL, ppc64ModelParse, map) < 0) - goto error; - - return map; + return NULL; - error: - ppc64MapFree(map); - return NULL; + return g_steal_pointer(&map); } static virCPUDataPtr @@ -428,7 +424,7 @@ ppc64Compute(virCPUDefPtr host, virCPUDataPtr *guestData, char **message) { - virCPUppc64MapPtr map = NULL; + g_autoptr(virCPUppc64Map) map = NULL; g_autoptr(virCPUppc64Model) host_model = NULL; g_autoptr(virCPUppc64Model) guest_model = NULL; virCPUDefPtr cpu = NULL; @@ -542,7 +538,6 @@ ppc64Compute(virCPUDefPtr host, cleanup: virCPUDefFree(cpu); - ppc64MapFree(map); return ret; } @@ -585,8 +580,7 @@ ppc64DriverDecode(virCPUDefPtr cpu, const virCPUData *data, virDomainCapsCPUModelsPtr models) { - int ret = -1; - virCPUppc64MapPtr map; + g_autoptr(virCPUppc64Map) map = NULL; const virCPUppc64Model *model; if (!data || !(map = ppc64LoadMap())) @@ -596,26 +590,21 @@ ppc64DriverDecode(virCPUDefPtr cpu, virReportError(VIR_ERR_OPERATION_FAILED, _("Cannot find CPU model with PVR 0x%08x"), data->data.ppc64.pvr[0].value); - goto cleanup; + return -1; } if (!virCPUModelIsAllowed(model->name, models)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("CPU model %s is not supported by hypervisor"), model->name); - goto cleanup; + return -1; } cpu->model = g_strdup(model->name); if (model->vendor) cpu->vendor = g_strdup(model->vendor->name); - ret = 0; - - cleanup: - ppc64MapFree(map); - - return ret; + return 0; } static void @@ -686,7 +675,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, const char **features G_GNUC_UNUSED, bool migratable G_GNUC_UNUSED) { - virCPUppc64MapPtr map; + g_autoptr(virCPUppc64Map) map = NULL; const virCPUppc64Model *model; const virCPUppc64Vendor *vendor = NULL; virCPUDefPtr cpu = NULL; @@ -761,23 +750,19 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, cpu->match = VIR_CPU_MATCH_EXACT; cpu->fallback = VIR_CPU_FALLBACK_FORBID; - cleanup: - ppc64MapFree(map); - return cpu; error: virCPUDefFree(cpu); cpu = NULL; - goto cleanup; + return NULL; } static int virCPUppc64DriverGetModels(char ***models) { - virCPUppc64MapPtr map; + g_autoptr(virCPUppc64Map) map = NULL; size_t i; - int ret = -1; if (!(map = ppc64LoadMap())) goto error; @@ -790,18 +775,14 @@ virCPUppc64DriverGetModels(char ***models) (*models)[i] = g_strdup(map->models[i]->name); } - ret = map->nmodels; - - cleanup: - ppc64MapFree(map); - return ret; + return map->nmodels; error: if (models) { g_strfreev(*models); *models = NULL; } - goto cleanup; + return -1; } struct cpuArchDriver cpuDriverPPC64 = { -- 2.26.2
s/autopr/autoptr/ On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Use autocleanup with virCPUppc64MapPtr to simplify existing code. Remove labels when possible.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 47 ++++++++++++++------------------------------- 1 file changed, 14 insertions(+), 33 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We don't need to call virCPUppc64DataFree() in a cleanup label. This function is already assigned to the 'dataFree' interface of cpuDriverPPC64, and it will be called by virCPUDataFree(), the autocleanup function of virCPUDataPtr, via driver->dataFree. Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index b7c4864200..bc9d399939 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -622,17 +622,16 @@ static int virCPUppc64GetHost(virCPUDefPtr cpu, virDomainCapsCPUModelsPtr models) { - virCPUDataPtr cpuData = NULL; + g_autoptr(virCPUData) cpuData = NULL; virCPUppc64Data *data; - int ret = -1; if (!(cpuData = virCPUDataNew(archs[0]))) - goto cleanup; + return -1; data = &cpuData->data.ppc64; if (VIR_ALLOC_N(data->pvr, 1) < 0) - goto cleanup; + return -1; data->len = 1; @@ -642,11 +641,7 @@ virCPUppc64GetHost(virCPUDefPtr cpu, #endif data->pvr[0].mask = 0xfffffffful; - ret = ppc64DriverDecode(cpu, cpuData, models); - - cleanup: - virCPUppc64DataFree(cpuData); - return ret; + return ppc64DriverDecode(cpu, cpuData, models); } -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
We don't need to call virCPUppc64DataFree() in a cleanup label. This function is already assigned to the 'dataFree' interface of cpuDriverPPC64, and it will be called by virCPUDataFree(), the autocleanup function of virCPUDataPtr, via driver->dataFree.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
This allows for a label removal in ppc64ModelParse(). Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index bc9d399939..0c140b33e4 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -308,22 +308,21 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, { virCPUppc64MapPtr map = data; g_autoptr(virCPUppc64Model) model = NULL; - xmlNodePtr *nodes = NULL; - char *vendor = NULL; + g_autofree xmlNodePtr *nodes = NULL; + g_autofree char *vendor = NULL; unsigned long pvr; size_t i; int n; - int ret = -1; if (VIR_ALLOC(model) < 0) - goto cleanup; + return -1; model->name = g_strdup(name); if (ppc64ModelFind(map, model->name)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("CPU model %s already defined"), model->name); - goto cleanup; + return -1; } if (virXPathBoolean("boolean(./vendor)", ctxt)) { @@ -332,14 +331,14 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid vendor element in CPU model %s"), model->name); - goto cleanup; + return -1; } if (!(model->vendor = ppc64VendorFind(map, vendor))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown vendor %s referenced by CPU model %s"), vendor, model->name); - goto cleanup; + return -1; } } @@ -347,11 +346,11 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing PVR information for CPU model %s"), model->name); - goto cleanup; + return -1; } if (VIR_ALLOC_N(model->data.pvr, n) < 0) - goto cleanup; + return -1; model->data.len = n; @@ -362,7 +361,7 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing or invalid PVR value in CPU model %s"), model->name); - goto cleanup; + return -1; } model->data.pvr[i].value = pvr; @@ -370,20 +369,15 @@ ppc64ModelParse(xmlXPathContextPtr ctxt, virReportError(VIR_ERR_INTERNAL_ERROR, _("Missing or invalid PVR mask in CPU model %s"), model->name); - goto cleanup; + return -1; } model->data.pvr[i].mask = pvr; } if (VIR_APPEND_ELEMENT(map->models, map->nmodels, model) < 0) - goto cleanup; - - ret = 0; + return -1; - cleanup: - VIR_FREE(vendor); - VIR_FREE(nodes); - return ret; + return 0; } @@ -547,7 +541,7 @@ virCPUppc64Compare(virCPUDefPtr host, bool failIncompatible) { virCPUCompareResult ret; - char *message = NULL; + g_autofree char *message = NULL; if (!host || !host->model) { if (failIncompatible) { @@ -570,7 +564,6 @@ virCPUppc64Compare(virCPUDefPtr host, virReportError(VIR_ERR_CPU_INCOMPATIBLE, NULL); } } - VIR_FREE(message); return ret; } -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
This allows for a label removal in ppc64ModelParse().
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 33 +++++++++++++-------------------- 1 file changed, 13 insertions(+), 20 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Using g_autoptr() in virCPUDef pointers allows for more cleanups in ppc64Compute() and virCPUppc64Baseline() Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 55 +++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 35 deletions(-) diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 0c140b33e4..9e6f1e856e 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -421,15 +421,14 @@ ppc64Compute(virCPUDefPtr host, g_autoptr(virCPUppc64Map) map = NULL; g_autoptr(virCPUppc64Model) host_model = NULL; g_autoptr(virCPUppc64Model) guest_model = NULL; - virCPUDefPtr cpu = NULL; - virCPUCompareResult ret = VIR_CPU_COMPARE_ERROR; + g_autoptr(virCPUDef) cpu = NULL; virArch arch; size_t i; /* Ensure existing configurations are handled correctly */ if (!(cpu = virCPUDefCopy(other)) || virCPUppc64ConvertLegacy(cpu) < 0) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; if (cpu->arch != VIR_ARCH_NONE) { bool found = false; @@ -448,8 +447,7 @@ ppc64Compute(virCPUDefPtr host, *message = g_strdup_printf(_("CPU arch %s does not match host arch"), virArchToString(cpu->arch)); - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - goto cleanup; + return VIR_CPU_COMPARE_INCOMPATIBLE; } arch = cpu->arch; } else { @@ -466,16 +464,15 @@ ppc64Compute(virCPUDefPtr host, cpu->vendor); } - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - goto cleanup; + return VIR_CPU_COMPARE_INCOMPATIBLE; } if (!(map = ppc64LoadMap())) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; /* Host CPU information */ if (!(host_model = ppc64ModelFromCPU(host, map))) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; if (cpu->type == VIR_CPU_TYPE_GUEST) { /* Guest CPU information */ @@ -485,10 +482,8 @@ ppc64Compute(virCPUDefPtr host, /* host-model only: * we need to take compatibility modes into account */ tmp = ppc64CheckCompatibilityMode(host->model, cpu->model); - if (tmp != VIR_CPU_COMPARE_IDENTICAL) { - ret = tmp; - goto cleanup; - } + if (tmp != VIR_CPU_COMPARE_IDENTICAL) + return tmp; G_GNUC_FALLTHROUGH; case VIR_CPU_MODE_HOST_PASSTHROUGH: @@ -509,7 +504,7 @@ ppc64Compute(virCPUDefPtr host, } if (!guest_model) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; if (STRNEQ(guest_model->name, host_model->name)) { VIR_DEBUG("host CPU model does not match required CPU model %s", @@ -520,19 +515,14 @@ ppc64Compute(virCPUDefPtr host, guest_model->name); } - ret = VIR_CPU_COMPARE_INCOMPATIBLE; - goto cleanup; + return VIR_CPU_COMPARE_INCOMPATIBLE; } if (guestData) if (!(*guestData = ppc64MakeCPUData(arch, &guest_model->data))) - goto cleanup; + return VIR_CPU_COMPARE_ERROR; - ret = VIR_CPU_COMPARE_IDENTICAL; - - cleanup: - virCPUDefFree(cpu); - return ret; + return VIR_CPU_COMPARE_IDENTICAL; } static virCPUCompareResult @@ -666,16 +656,16 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, g_autoptr(virCPUppc64Map) map = NULL; const virCPUppc64Model *model; const virCPUppc64Vendor *vendor = NULL; - virCPUDefPtr cpu = NULL; + g_autoptr(virCPUDef) cpu = NULL; size_t i; if (!(map = ppc64LoadMap())) - goto error; + return NULL; if (!(model = ppc64ModelFind(map, cpus[0]->model))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown CPU model %s"), cpus[0]->model); - goto error; + return NULL; } for (i = 0; i < ncpus; i++) { @@ -695,7 +685,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, if (STRNEQ(cpus[i]->model, model->name)) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("CPUs are incompatible")); - goto error; + return NULL; } if (!cpus[i]->vendor) @@ -704,7 +694,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, if (!(vnd = ppc64VendorFind(map, cpus[i]->vendor))) { virReportError(VIR_ERR_OPERATION_FAILED, _("Unknown CPU vendor %s"), cpus[i]->vendor); - goto error; + return NULL; } if (model->vendor) { @@ -714,13 +704,13 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, "vendor %s"), model->vendor->name, model->name, vnd->name); - goto error; + return NULL; } } else if (vendor) { if (vendor != vnd) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("CPU vendors do not match")); - goto error; + return NULL; } } else { vendor = vnd; @@ -738,12 +728,7 @@ virCPUppc64Baseline(virCPUDefPtr *cpus, cpu->match = VIR_CPU_MATCH_EXACT; cpu->fallback = VIR_CPU_FALLBACK_FORBID; - return cpu; - - error: - virCPUDefFree(cpu); - cpu = NULL; - return NULL; + return g_steal_pointer(&cpu); } static int -- 2.26.2
On a Wednesday in 2020, Daniel Henrique Barboza wrote:
Using g_autoptr() in virCPUDef pointers allows for more cleanups in ppc64Compute() and virCPUppc64Baseline()
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> --- src/cpu/cpu_ppc64.c | 55 +++++++++++++++++---------------------------- 1 file changed, 20 insertions(+), 35 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Daniel Henrique Barboza -
Ján Tomko