[libvirt PATCH 0/7] Replace VIR_ALLOC and VIR_FREE in src/cpu/*.
This series furthers the transition from VIR_ALLOC, VIR_FREE, etc to glib fun= ctions and removes the "_QUIET" variants of VIR_{ALLOC,ALLOC_N,REALLOC_N,APPEND_ELEMENT}= that by now are aliases to their non-quiet counterparts. Tim Wiederhake (7): util: alloc: Remove VIR_ALLOC_QUIET util: alloc: Remove VIR_ALLOC_N_QUIET util: alloc: Remove VIR_REALLOC_N_QUIET util: alloc: Remove VIR_APPEND_ELEMENT_QUIET 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 | 6 ++--- src/cpu/cpu_ppc64.c | 59 +++++++++++++++-------------------------- src/cpu/cpu_x86.c | 10 +++---- src/qemu/qemu_domain.c | 4 +-- src/qemu/qemu_process.c | 2 +- src/rpc/virnetmessage.c | 2 +- src/util/viralloc.h | 48 --------------------------------- src/util/virbitmap.c | 4 +-- src/util/virdevmapper.c | 2 +- src/util/virerror.c | 6 ++--- src/util/virfile.c | 2 +- src/util/virjson.c | 2 +- src/util/virlog.c | 4 +-- src/util/virthread.c | 2 +- tests/virconftest.c | 2 +- tests/virfilewrapper.c | 4 +-- tests/virpcimock.c | 19 +++++++------ tools/vsh.c | 2 +- 19 files changed, 59 insertions(+), 127 deletions(-) --=20 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/rpc/virnetmessage.c | 2 +- src/util/viralloc.h | 14 -------------- src/util/virbitmap.c | 2 +- src/util/virerror.c | 6 +++--- src/util/virlog.c | 2 +- src/util/virthread.c | 2 +- tests/virpcimock.c | 6 +++--- 7 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index c4ddafc01d..011b32b100 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -539,7 +539,7 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) } else { rerr->code = VIR_ERR_INTERNAL_ERROR; rerr->domain = VIR_FROM_RPC; - if (VIR_ALLOC_QUIET(rerr->message) == 0) + if (VIR_ALLOC(rerr->message) == 0) *rerr->message = g_strdup(_("Library function returned error but did not set virError")); rerr->level = VIR_ERR_ERROR; } diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 833f85f62e..ae967f2556 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -75,20 +75,6 @@ void virDisposeString(char **strptr) */ #define VIR_ALLOC(ptr) virAlloc(&(ptr), sizeof(*(ptr))) -/** - * VIR_ALLOC_QUIET: - * @ptr: pointer to hold address of allocated memory - * - * Allocate sizeof(*ptr) bytes of memory and store - * the address of allocated memory in 'ptr'. Fill the - * newly allocated memory with zeros. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_ALLOC_QUIET(ptr) VIR_ALLOC(ptr) - /** * VIR_ALLOC_N: * @ptr: pointer to hold address of allocated memory diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 60fd8491dd..7198bfa66e 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -69,7 +69,7 @@ virBitmapNewQuiet(size_t size) sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT); - if (VIR_ALLOC_QUIET(bitmap) < 0) + if (VIR_ALLOC(bitmap) < 0) return NULL; if (VIR_ALLOC_N_QUIET(bitmap->map, sz) < 0) { diff --git a/src/util/virerror.c b/src/util/virerror.c index 507a29f50f..fc9e10c649 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -225,7 +225,7 @@ virErrorCopyNew(virErrorPtr err) { virErrorPtr ret; - if (VIR_ALLOC_QUIET(ret) < 0) + if (VIR_ALLOC(ret) < 0) return NULL; if (virCopyError(err, ret) < 0) @@ -241,7 +241,7 @@ virLastErrorObject(void) virErrorPtr err; err = virThreadLocalGet(&virLastErr); if (!err) { - if (VIR_ALLOC_QUIET(err) < 0) + if (VIR_ALLOC(err) < 0) return NULL; if (virThreadLocalSet(&virLastErr, err) < 0) VIR_FREE(err); @@ -404,7 +404,7 @@ virSaveLastError(void) virErrorPtr to; int saved_errno = errno; - if (VIR_ALLOC_QUIET(to) < 0) + if (VIR_ALLOC(to) < 0) return NULL; virCopyLastError(to); diff --git a/src/util/virlog.c b/src/util/virlog.c index f6d0c6c050..b368ce23f3 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1327,7 +1327,7 @@ virLogFilterNew(const char *match, memcpy(mdup + 1, match, mlen); mdup[mlen + 1] = '*'; - if (VIR_ALLOC_QUIET(ret) < 0) { + if (VIR_ALLOC(ret) < 0) { VIR_FREE(mdup); return NULL; } diff --git a/src/util/virthread.c b/src/util/virthread.c index 7f23399835..c35b918c17 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -253,7 +253,7 @@ int virThreadCreateFull(virThreadPtr thread, if ((err = pthread_attr_init(&attr)) != 0) goto cleanup; - if (VIR_ALLOC_QUIET(args) < 0) { + if (VIR_ALLOC(args) < 0) { err = ENOMEM; goto cleanup; } diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 92b6f810d8..76818d0031 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -427,7 +427,7 @@ pci_device_create_iommu(const struct pciDevice *dev, return; } - if (VIR_ALLOC_QUIET(iommuGroup) < 0) + if (VIR_ALLOC(iommuGroup) < 0) ABORT_OOM(); iommuGroup->iommu = dev->iommuGroup; @@ -469,7 +469,7 @@ pci_device_new_from_stub(const struct pciDevice *data) c = strchr(c, ':'); } - if (VIR_ALLOC_QUIET(dev) < 0) + if (VIR_ALLOC(dev) < 0) ABORT_OOM(); configSrc = g_strdup_printf("%s/virpcitestdata/%s.config", abs_srcdir, id); @@ -694,7 +694,7 @@ pci_driver_new(const char *name, ...) int vendor, device; g_autofree char *driverpath = NULL; - if (VIR_ALLOC_QUIET(driver) < 0) + if (VIR_ALLOC(driver) < 0) ABORT_OOM(); driver->name = g_strdup(name); if (!(driverpath = pci_driver_get_path(driver, NULL, true))) -- 2.26.2
On Thu, Sep 10, 2020 at 01:29:19PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/rpc/virnetmessage.c | 2 +- src/util/viralloc.h | 14 -------------- src/util/virbitmap.c | 2 +- src/util/virerror.c | 6 +++--- src/util/virlog.c | 2 +- src/util/virthread.c | 2 +- tests/virpcimock.c | 6 +++--- 7 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index c4ddafc01d..011b32b100 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -539,7 +539,7 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) } else { rerr->code = VIR_ERR_INTERNAL_ERROR; rerr->domain = VIR_FROM_RPC; - if (VIR_ALLOC_QUIET(rerr->message) == 0) + if (VIR_ALLOC(rerr->message) == 0)
We didn't want to be doing such conversions. We kept VIR_ALLOC_QUIET as an alias for VIR_ALLOC, because the intent was to convert straight to g_new0, and not have this intermediate step. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, 2020-09-10 at 12:36 +0100, Daniel P. Berrangé wrote:
On Thu, Sep 10, 2020 at 01:29:19PM +0200, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/rpc/virnetmessage.c | 2 +- src/util/viralloc.h | 14 -------------- src/util/virbitmap.c | 2 +- src/util/virerror.c | 6 +++--- src/util/virlog.c | 2 +- src/util/virthread.c | 2 +- tests/virpcimock.c | 6 +++--- 7 files changed, 10 insertions(+), 24 deletions(-)
diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index c4ddafc01d..011b32b100 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -539,7 +539,7 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) } else { rerr->code = VIR_ERR_INTERNAL_ERROR; rerr->domain = VIR_FROM_RPC; - if (VIR_ALLOC_QUIET(rerr->message) == 0) + if (VIR_ALLOC(rerr->message) == 0)
We didn't want to be doing such conversions. We kept VIR_ALLOC_QUIET as an alias for VIR_ALLOC, because the intent was to convert straight to g_new0, and not have this intermediate step.
Thanks for the input! I will split the series and rework the "alias removal" part. Tim
Regards, Daniel
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 15 --------------- src/util/virbitmap.c | 2 +- src/util/virdevmapper.c | 2 +- src/util/virjson.c | 2 +- src/util/virlog.c | 2 +- tests/virconftest.c | 2 +- 6 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index ae967f2556..c60148724d 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -90,21 +90,6 @@ void virDisposeString(char **strptr) */ #define VIR_ALLOC_N(ptr, count) virAllocN(&(ptr), sizeof(*(ptr)), (count)) -/** - * VIR_ALLOC_N_QUIET: - * @ptr: pointer to hold address of allocated memory - * @count: number of elements to allocate - * - * Allocate an array of 'count' elements, each sizeof(*ptr) - * bytes long and store the address of allocated memory in - * 'ptr'. Fill the newly allocated memory with zeros. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_ALLOC_N_QUIET(ptr, count) VIR_ALLOC_N(ptr, count) - /** * VIR_REALLOC_N: * @ptr: pointer to hold address of allocated memory diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 7198bfa66e..35ca914ce2 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -72,7 +72,7 @@ virBitmapNewQuiet(size_t size) if (VIR_ALLOC(bitmap) < 0) return NULL; - if (VIR_ALLOC_N_QUIET(bitmap->map, sz) < 0) { + if (VIR_ALLOC_N(bitmap->map, sz) < 0) { VIR_FREE(bitmap); return NULL; } diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c index 7c89c2a952..3c45d3b7f5 100644 --- a/src/util/virdevmapper.c +++ b/src/util/virdevmapper.c @@ -260,7 +260,7 @@ virDevMapperGetTargetsImpl(int controlFD, return -1; } - if (VIR_ALLOC_N_QUIET(devPaths, deps->count + 1) < 0) + if (VIR_ALLOC_N(devPaths, deps->count + 1) < 0) return -1; for (i = 0; i < deps->count; i++) { diff --git a/src/util/virjson.c b/src/util/virjson.c index 6921eccb60..cdaca20628 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1243,7 +1243,7 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val, if (val->type != VIR_JSON_TYPE_ARRAY) return -1; - if (VIR_ALLOC_N_QUIET(elems, val->data.array.nvalues) < 0) + if (VIR_ALLOC_N(elems, val->data.array.nvalues) < 0) return -1; /* first pass converts array members to numbers and finds the maximum */ diff --git a/src/util/virlog.c b/src/util/virlog.c index b368ce23f3..677ba1dfac 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1320,7 +1320,7 @@ virLogFilterNew(const char *match, /* We must treat 'foo' as equiv to '*foo*' for g_pattern_match * todo substring matches, so add 2 extra bytes */ - if (VIR_ALLOC_N_QUIET(mdup, mlen + 3) < 0) + if (VIR_ALLOC_N(mdup, mlen + 3) < 0) return NULL; mdup[0] = '*'; diff --git a/tests/virconftest.c b/tests/virconftest.c index ab29b5b712..3ae4d4ecc0 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -42,7 +42,7 @@ static int testConfRoundTrip(const void *opaque) srcfile = g_strdup_printf("%s/virconfdata/%s.conf", abs_srcdir, name); dstfile = g_strdup_printf("%s/virconfdata/%s.out", abs_srcdir, name); - if (VIR_ALLOC_N_QUIET(buffer, len) < 0) { + if (VIR_ALLOC_N(buffer, len) < 0) { fprintf(stderr, "out of memory\n"); goto cleanup; } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_process.c | 2 +- src/util/viralloc.h | 15 --------------- src/util/virfile.c | 2 +- tests/virpcimock.c | 6 +++--- tools/vsh.c | 2 +- 6 files changed, 8 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0ed132a829..9719636a2a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -511,7 +511,7 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) goto error; } - ignore_value(VIR_REALLOC_N_QUIET(masterKey, masterKeyLen)); + ignore_value(VIR_REALLOC_N(masterKey, masterKeyLen)); priv->masterKey = masterKey; priv->masterKeyLen = masterKeyLen; @@ -6357,7 +6357,7 @@ ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt, buf[got] = '\0'; - ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1)); + ignore_value(VIR_REALLOC_N(buf, got + 1)); buflen = got; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index dd60fb0ddf..42a5d3ce19 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2082,7 +2082,7 @@ qemuProcessReadLog(qemuDomainLogContextPtr logCtxt, got -= skip; } - ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1)); + ignore_value(VIR_REALLOC_N(buf, got + 1)); *msg = buf; return 0; } diff --git a/src/util/viralloc.h b/src/util/viralloc.h index c60148724d..a50cd5d632 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -105,21 +105,6 @@ void virDisposeString(char **strptr) */ #define VIR_REALLOC_N(ptr, count) virReallocN(&(ptr), sizeof(*(ptr)), (count)) -/** - * VIR_REALLOC_N_QUIET: - * @ptr: pointer to hold address of allocated memory - * @count: number of elements to allocate - * - * Re-allocate an array of 'count' elements, each sizeof(*ptr) - * bytes long and store the address of allocated memory in - * 'ptr'. If 'ptr' grew, the added memory is uninitialized. - * - * This macro is safe to use on arguments with side effects. - * - * Returns 0 on success, aborts on OOM - */ -#define VIR_REALLOC_N_QUIET(ptr, count) VIR_REALLOC_N(ptr, count) - /** * VIR_EXPAND_N: * @ptr: pointer to hold address of allocated memory diff --git a/src/util/virfile.c b/src/util/virfile.c index 90156845df..716ffdf7b6 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -4339,7 +4339,7 @@ virFileGetXAttrQuiet(const char *path, if ((need = getxattr(path, name, NULL, 0)) < 0) goto cleanup; - if (VIR_REALLOC_N_QUIET(buf, need + 1) < 0) + if (VIR_REALLOC_N(buf, need + 1) < 0) goto cleanup; if ((got = getxattr(path, name, buf, need)) < 0) { diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 76818d0031..30a31cb5de 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -305,7 +305,7 @@ add_fd(int fd, const char *path) fd, path, cb.fd, cb.path); } - if (VIR_REALLOC_N_QUIET(callbacks, nCallbacks + 1) < 0) { + if (VIR_REALLOC_N(callbacks, nCallbacks + 1) < 0) { errno = ENOMEM; return -1; } @@ -709,8 +709,8 @@ pci_driver_new(const char *name, ...) if ((device = va_arg(args, int)) == -1) ABORT("Invalid vendor device pair for driver %s", name); - if (VIR_REALLOC_N_QUIET(driver->vendor, driver->len + 1) < 0 || - VIR_REALLOC_N_QUIET(driver->device, driver->len + 1) < 0) + if (VIR_REALLOC_N(driver->vendor, driver->len + 1) < 0 || + VIR_REALLOC_N(driver->device, driver->len + 1) < 0) ABORT_OOM(); driver->vendor[driver->len] = vendor; diff --git a/tools/vsh.c b/tools/vsh.c index 0e8edcd78c..345458a0e2 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2723,7 +2723,7 @@ vshCompleterFilter(char ***list, newList_len++; } - ignore_value(VIR_REALLOC_N_QUIET(newList, newList_len + 1)); + ignore_value(VIR_REALLOC_N(newList, newList_len + 1)); VIR_FREE(*list); *list = newList; return 0; -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 4 ---- tests/virfilewrapper.c | 4 ++-- tests/virpcimock.c | 7 +++---- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/util/viralloc.h b/src/util/viralloc.h index a50cd5d632..a581a2935e 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -303,10 +303,6 @@ void virDisposeString(char **strptr) VIR_TYPEMATCH(ptr, &(newelem)), \ &(newelem), false, true)) -/* Quiet version of macros above */ -#define VIR_APPEND_ELEMENT_QUIET(ptr, count, newelem) \ - VIR_APPEND_ELEMENT(ptr, count, newelem) - /** * VIR_DELETE_ELEMENT: * @ptr: pointer to array of objects (*not* ptr to ptr) diff --git a/tests/virfilewrapper.c b/tests/virfilewrapper.c index 0500a3617e..ca7274bfe7 100644 --- a/tests/virfilewrapper.c +++ b/tests/virfilewrapper.c @@ -74,8 +74,8 @@ virFileWrapperAddPrefix(const char *prefix, init_syms(); - if (VIR_APPEND_ELEMENT_QUIET(prefixes, nprefixes, prefix) < 0 || - VIR_APPEND_ELEMENT_QUIET(overrides, noverrides, override) < 0) { + if (VIR_APPEND_ELEMENT(prefixes, nprefixes, prefix) < 0 || + VIR_APPEND_ELEMENT(overrides, noverrides, override) < 0) { VIR_FREE(prefixes); VIR_FREE(overrides); fprintf(stderr, "Unable to add path override for '%s'\n", prefix); diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 30a31cb5de..b29f34bc4f 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -433,8 +433,7 @@ pci_device_create_iommu(const struct pciDevice *dev, iommuGroup->iommu = dev->iommuGroup; iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to VFIO by default */ - if (VIR_APPEND_ELEMENT_QUIET(pciIommuGroups, npciIommuGroups, - iommuGroup) < 0) + if (VIR_APPEND_ELEMENT(pciIommuGroups, npciIommuGroups, iommuGroup) < 0) ABORT_OOM(); } @@ -550,7 +549,7 @@ pci_device_new_from_stub(const struct pciDevice *data) if (pci_device_autobind(dev) < 0) ABORT("Unable to bind: %s", devid); - if (VIR_APPEND_ELEMENT_QUIET(pciDevices, nPCIDevices, dev) < 0) + if (VIR_APPEND_ELEMENT(pciDevices, nPCIDevices, dev) < 0) ABORT_OOM(); } @@ -723,7 +722,7 @@ pci_driver_new(const char *name, ...) make_file(driverpath, "bind", NULL, -1); make_file(driverpath, "unbind", NULL, -1); - if (VIR_APPEND_ELEMENT_QUIET(pciDrivers, nPCIDrivers, driver) < 0) + if (VIR_APPEND_ELEMENT(pciDrivers, nPCIDrivers, driver) < 0) ABORT_OOM(); } -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 4 +--- src/cpu/cpu_ppc64.c | 25 ++++++++----------------- 2 files changed, 9 insertions(+), 20 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 28fbfea9ae..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; @@ -399,17 +392,15 @@ static virCPUDataPtr ppc64MakeCPUData(virArch arch, virCPUppc64Data *data) { - virCPUDataPtr cpuData; - - if (VIR_ALLOC(cpuData) < 0) - return NULL; + g_autoptr(virCPUData) cpuData = NULL; + cpuData = g_new0(virCPUData, 1); 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 Thursday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 4 +--- src/cpu/cpu_ppc64.c | 25 ++++++++----------------- 2 files changed, 9 insertions(+), 20 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 @@ -399,17 +392,15 @@ static virCPUDataPtr ppc64MakeCPUData(virArch arch, virCPUppc64Data *data) { - virCPUDataPtr cpuData; - - if (VIR_ALLOC(cpuData) < 0) - return NULL; + g_autoptr(virCPUData) cpuData = NULL;
+ cpuData = g_new0(virCPUData, 1); cpuData->arch = arch;
if (ppc64DataCopy(&cpuData->data.ppc64, data) < 0) - VIR_FREE(cpuData); + return NULL;
- return cpuData; + return g_steal_pointer(&cpuData);
This combines the VIR_ALLOC -> g_new0 replacement with other refactors. For neat separation of changes into commits, it would be nicer to have the other changes separate. But I could live with the changes being here, if you mention them in the commit message. Jano
}
static virCPUCompareResult -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@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
On a Thursday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu_ppc64.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 2 +- src/cpu/cpu_map.c | 6 +++--- src/cpu/cpu_ppc64.c | 18 +++++++++--------- src/cpu/cpu_x86.c | 10 +++++----- 4 files changed, 18 insertions(+), 18 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_map.c b/src/cpu/cpu_map.c index 65d244e011..7859eef9aa 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -64,7 +64,7 @@ loadData(const char *mapfile, VIR_DEBUG("Load %s name %s", element, name); ctxt->node = nodes[i]; rv = callback(ctxt, name, data); - VIR_FREE(name); + g_free(name); if (rv < 0) return -1; } @@ -133,10 +133,10 @@ loadIncludes(xmlXPathContextPtr ctxt, } VIR_DEBUG("Finding CPU map include '%s'", filename); if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) { - VIR_FREE(filename); + g_free(filename); return -1; } - VIR_FREE(filename); + g_free(filename); } return 0; diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6477b4bce7..7a36981e12 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_free(data->pvr); } 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 fdb665b01d..6a46e186ce 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); } @@ -1805,7 +1805,7 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt) if (message) \ *message = g_strdup_printf("%s: %s", _(MSG), flagsStr); \ VIR_DEBUG("%s: %s", MSG, flagsStr); \ - VIR_FREE(flagsStr); \ + g_free(flagsStr); \ } while (0) @@ -2208,7 +2208,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); } @@ -2893,7 +2893,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); } @@ -2915,7 +2915,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 Thursday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/cpu/cpu.c | 2 +- src/cpu/cpu_map.c | 6 +++--- src/cpu/cpu_ppc64.c | 18 +++++++++--------- src/cpu/cpu_x86.c | 10 +++++----- 4 files changed, 18 insertions(+), 18 deletions(-)
[...]
diff --git a/src/cpu/cpu_map.c b/src/cpu/cpu_map.c index 65d244e011..7859eef9aa 100644 --- a/src/cpu/cpu_map.c +++ b/src/cpu/cpu_map.c @@ -64,7 +64,7 @@ loadData(const char *mapfile, VIR_DEBUG("Load %s name %s", element, name); ctxt->node = nodes[i]; rv = callback(ctxt, name, data); - VIR_FREE(name); + g_free(name); if (rv < 0) return -1; } @@ -133,10 +133,10 @@ loadIncludes(xmlXPathContextPtr ctxt, } VIR_DEBUG("Finding CPU map include '%s'", filename); if (cpuMapLoadInclude(filename, vendorCB, featureCB, modelCB, data) < 0) { - VIR_FREE(filename); + g_free(filename); return -1; } - VIR_FREE(filename); + g_free(filename); }
return 0;
The cases above can be converted to g_autofree
diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index 6477b4bce7..7a36981e12 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_free(data->pvr); }
g_clear_pointer. This is a *Clear function and we should not leave stale pointers around in those.
@@ -1805,7 +1805,7 @@ virCPUx86DataParse(xmlXPathContextPtr ctxt) if (message) \ *message = g_strdup_printf("%s: %s", _(MSG), flagsStr); \ VIR_DEBUG("%s: %s", MSG, flagsStr); \ - VIR_FREE(flagsStr); \ + g_free(flagsStr); \ } while (0)
This one can also use g_autofree Jano
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Tim Wiederhake