[libvirt PATCH 00/21] Remove VIR_{ALLOC, ALLOC_N, REALLOC_N}_QUIET macros.
See also https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html and https://www.redhat.com/archives/libvir-list/2020-September/msg00610.html Tim Wiederhake (21): tests: Use glib memory functions in virpcimock.c rpc: Use glib memory functions in virNetMessageSaveError util: Use glib memory functions in virBitmapNewQuiet util: Use glib memory functions in virErrorCopyNew util: Use glib memory functions in virLastErrorObject util: Use glib memory functions in virSaveLastError util: Use glib memory functions in virLogFilterNew util: Use glib memory functions in virThreadCreateFull util: Remove VIR_ALLOC_QUIET tests: Use glib memory function in testConfRoundTrip util: Use glib memory functions in virDevMapperGetTargetsImpl util: Use glib memory functions in virJSONValueGetArrayAsBitmap util: Remove VIR_ALLOC_N_QUIET qemu: Use glib memory functions in qemuDomainMasterKeyReadFile qemu: Use glib memory functions in qemuDomainLogContextRead qemu: Use glib memory functions in qemuProcessReadLog tests: Use glib memory functions in add_fd tests: Use glib memory functions in pci_driver_new tools: Use glib memory functions in vshCompleterFilter util: Use glib memory functions in virFileGetXAttrQuiet util: Remove VIR_REALLOC_N_QUIET src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_process.c | 2 +- src/rpc/virnetmessage.c | 4 ++-- src/util/viralloc.h | 44 ----------------------------------------- src/util/virbitmap.c | 10 ++-------- src/util/virdevmapper.c | 4 +--- src/util/virerror.c | 30 ++++++++++++++-------------- src/util/virfile.c | 15 +++++--------- src/util/virjson.c | 4 ++-- src/util/virlog.c | 24 ++++++++-------------- src/util/virthread.c | 5 +---- tests/virconftest.c | 29 ++++++++++----------------- tests/virpcimock.c | 24 ++++++++-------------- tools/vsh.c | 10 ++++------ 14 files changed, 61 insertions(+), 148 deletions(-) -- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virpcimock.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 92b6f810d8..6b1f2f2a5a 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -427,9 +427,7 @@ pci_device_create_iommu(const struct pciDevice *dev, return; } - if (VIR_ALLOC_QUIET(iommuGroup) < 0) - ABORT_OOM(); - + iommuGroup = g_new0(struct pciIommuGroup, 1); iommuGroup->iommu = dev->iommuGroup; iommuGroup->nDevicesBoundToVFIO = 0; /* No device bound to VFIO by default */ @@ -469,8 +467,7 @@ pci_device_new_from_stub(const struct pciDevice *data) c = strchr(c, ':'); } - if (VIR_ALLOC_QUIET(dev) < 0) - ABORT_OOM(); + dev = g_new0(struct pciDevice, 1); configSrc = g_strdup_printf("%s/virpcitestdata/%s.config", abs_srcdir, id); @@ -694,8 +691,7 @@ pci_driver_new(const char *name, ...) int vendor, device; g_autofree char *driverpath = NULL; - if (VIR_ALLOC_QUIET(driver) < 0) - ABORT_OOM(); + driver = g_new0(struct pciDriver, 1); driver->name = g_strdup(name); if (!(driverpath = pci_driver_get_path(driver, NULL, true))) ABORT_OOM(); -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virpcimock.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/rpc/virnetmessage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetmessage.c b/src/rpc/virnetmessage.c index c4ddafc01d..6423ce67a3 100644 --- a/src/rpc/virnetmessage.c +++ b/src/rpc/virnetmessage.c @@ -539,8 +539,8 @@ void virNetMessageSaveError(virNetMessageErrorPtr rerr) } else { rerr->code = VIR_ERR_INTERNAL_ERROR; rerr->domain = VIR_FROM_RPC; - if (VIR_ALLOC_QUIET(rerr->message) == 0) - *rerr->message = g_strdup(_("Library function returned error but did not set virError")); + rerr->message = g_new0(virNetMessageNonnullString, 1); + *rerr->message = g_strdup(_("Library function returned error but did not set virError")); rerr->level = VIR_ERR_ERROR; } } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/rpc/virnetmessage.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virbitmap.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 60fd8491dd..639103e518 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -69,14 +69,8 @@ virBitmapNewQuiet(size_t size) sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT); - if (VIR_ALLOC_QUIET(bitmap) < 0) - return NULL; - - if (VIR_ALLOC_N_QUIET(bitmap->map, sz) < 0) { - VIR_FREE(bitmap); - return NULL; - } - + bitmap = g_new0(virBitmap, 1); + bitmap->map = g_new0(unsigned long, sz); bitmap->nbits = size; bitmap->map_len = sz; bitmap->map_alloc = sz; -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virbitmap.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 507a29f50f..4ba99defbb 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -223,15 +223,14 @@ virCopyError(virErrorPtr from, virErrorPtr virErrorCopyNew(virErrorPtr err) { - virErrorPtr ret; + g_autoptr(virError) ret = NULL; - if (VIR_ALLOC_QUIET(ret) < 0) - return NULL; + ret = g_new0(virError, 1); if (virCopyError(err, ret) < 0) - VIR_FREE(ret); + return NULL; - return ret; + return g_steal_pointer(&ret); } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c index 507a29f50f..4ba99defbb 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -223,15 +223,14 @@ virCopyError(virErrorPtr from, virErrorPtr virErrorCopyNew(virErrorPtr err) { - virErrorPtr ret; + g_autoptr(virError) ret = NULL;
Using g_auto is not mandatory, sometimes it adds more clutter to the code than it removes.
- if (VIR_ALLOC_QUIET(ret) < 0) - return NULL; + ret = g_new0(virError, 1);
if (virCopyError(err, ret) < 0) - VIR_FREE(ret);
As of commit 18f377178a3bee6df4c63283ff61b75c21bc0d52 virCopyError always returns 0, so this condition is pointless. virCopyError(err, ret); should be enough and it would remove the need for g_auto. Jano
+ return NULL;
- return ret; + return g_steal_pointer(&ret); }
-- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 4ba99defbb..99a0855b51 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -237,15 +237,17 @@ virErrorCopyNew(virErrorPtr err) static virErrorPtr virLastErrorObject(void) { - virErrorPtr err; + g_autoptr(virError) err = NULL; + err = virThreadLocalGet(&virLastErr); - if (!err) { - if (VIR_ALLOC_QUIET(err) < 0) - return NULL; - if (virThreadLocalSet(&virLastErr, err) < 0) - VIR_FREE(err); - } - return err; + if (err) + return g_steal_pointer(&err); + + err = g_new0(virError, 1); + if (virThreadLocalSet(&virLastErr, err) < 0) + return NULL; + + return g_steal_pointer(&err); } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/src/util/virerror.c b/src/util/virerror.c index 4ba99defbb..99a0855b51 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -237,15 +237,17 @@ virErrorCopyNew(virErrorPtr err) static virErrorPtr virLastErrorObject(void) { - virErrorPtr err; + g_autoptr(virError) err = NULL; +
This function would also look nicer without g_auto - most of the code paths don't want to free err. Jano
err = virThreadLocalGet(&virLastErr); - if (!err) { - if (VIR_ALLOC_QUIET(err) < 0) - return NULL; - if (virThreadLocalSet(&virLastErr, err) < 0) - VIR_FREE(err); - } - return err; + if (err) + return g_steal_pointer(&err); + + err = g_new0(virError, 1); + if (virThreadLocalSet(&virLastErr, err) < 0) + return NULL; + + return g_steal_pointer(&err); }
-- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 99a0855b51..df4205043a 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -405,8 +405,7 @@ virSaveLastError(void) virErrorPtr to; int saved_errno = errno; - if (VIR_ALLOC_QUIET(to) < 0) - return NULL; + to = g_new0(virError, 1); virCopyLastError(to); errno = saved_errno; -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.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/util/virlog.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index f6d0c6c050..de36da1a4a 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1308,7 +1308,6 @@ virLogFilterNew(const char *match, virLogPriority priority) { virLogFilterPtr ret = NULL; - char *mdup = NULL; size_t mlen = strlen(match); if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) { @@ -1317,23 +1316,16 @@ virLogFilterNew(const char *match, return NULL; } + ret = g_new0(virLogFilter, 1); + ret->priority = priority; + /* We must treat 'foo' as equiv to '*foo*' for g_pattern_match - * todo substring matches, so add 2 extra bytes + * substring matches, so add 2 extra bytes */ - if (VIR_ALLOC_N_QUIET(mdup, mlen + 3) < 0) - return NULL; - - mdup[0] = '*'; - memcpy(mdup + 1, match, mlen); - mdup[mlen + 1] = '*'; - - if (VIR_ALLOC_QUIET(ret) < 0) { - VIR_FREE(mdup); - return NULL; - } - - ret->match = mdup; - ret->priority = priority; + ret->match = g_new0(char, mlen + 3); + ret->match[0] = '*'; + memcpy(ret->match + 1, match, mlen); + ret->match[mlen + 1] = '*'; return ret; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virlog.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/src/util/virlog.c b/src/util/virlog.c index f6d0c6c050..de36da1a4a 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -1308,7 +1308,6 @@ virLogFilterNew(const char *match, virLogPriority priority) { virLogFilterPtr ret = NULL; - char *mdup = NULL; size_t mlen = strlen(match);
if (priority < VIR_LOG_DEBUG || priority > VIR_LOG_ERROR) { @@ -1317,23 +1316,16 @@ virLogFilterNew(const char *match, return NULL; }
+ ret = g_new0(virLogFilter, 1); + ret->priority = priority; +
/* We must treat 'foo' as equiv to '*foo*' for g_pattern_match - * todo substring matches, so add 2 extra bytes + * substring matches, so add 2 extra bytes */
Unrelated comment change.
- if (VIR_ALLOC_N_QUIET(mdup, mlen + 3) < 0) - return NULL; - - mdup[0] = '*'; - memcpy(mdup + 1, match, mlen); - mdup[mlen + 1] = '*'; - - if (VIR_ALLOC_QUIET(ret) < 0) { - VIR_FREE(mdup); - return NULL; - } - - ret->match = mdup; - ret->priority = priority; + ret->match = g_new0(char, mlen + 3); + ret->match[0] = '*'; + memcpy(ret->match + 1, match, mlen); + ret->match[mlen + 1] = '*';
return ret; }
With the comment change removed: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virthread.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/util/virthread.c b/src/util/virthread.c index 7f23399835..814f5313aa 100644 --- a/src/util/virthread.c +++ b/src/util/virthread.c @@ -253,11 +253,8 @@ int virThreadCreateFull(virThreadPtr thread, if ((err = pthread_attr_init(&attr)) != 0) goto cleanup; - if (VIR_ALLOC_QUIET(args) < 0) { - err = ENOMEM; - goto cleanup; - } + args = g_new0(struct virThreadArgs, 1); args->func = func; args->name = g_strdup(name); args->worker = worker; -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virthread.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 14 -------------- 1 file changed, 14 deletions(-) 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 -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 14 -------------- 1 file changed, 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
This also fixes a (hypothetical) bug where testConfRoundTrip would return 0 if virConfWriteMem() returned 0 and virTestCompareToFile failed. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virconftest.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/tests/virconftest.c b/tests/virconftest.c index ab29b5b712..c683344f49 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -32,40 +32,31 @@ static int testConfRoundTrip(const void *opaque) { const char *name = opaque; - int ret = -1; g_autoptr(virConf) conf = NULL; int len = 10000; - char *buffer = NULL; - char *srcfile = NULL; - char *dstfile = NULL; + g_autofree char *buffer = NULL; + g_autofree char *srcfile = NULL; + g_autofree char *dstfile = NULL; 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) { - fprintf(stderr, "out of memory\n"); - goto cleanup; - } + buffer = g_new0(char, len); conf = virConfReadFile(srcfile, 0); if (conf == NULL) { fprintf(stderr, "Failed to process %s\n", srcfile); - goto cleanup; + return -1; } - ret = virConfWriteMem(buffer, &len, conf); - if (ret < 0) { + + if (virConfWriteMem(buffer, &len, conf) < 0) { fprintf(stderr, "Failed to serialize %s back\n", srcfile); - goto cleanup; + return -1; } if (virTestCompareToFile(buffer, dstfile) < 0) - goto cleanup; + return -1; - ret = 0; - cleanup: - VIR_FREE(srcfile); - VIR_FREE(dstfile); - VIR_FREE(buffer); - return ret; + return 0; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
This also fixes a (hypothetical) bug where testConfRoundTrip would return 0 if virConfWriteMem() returned 0 and virTestCompareToFile failed.
The bug is not hypothetical, it renders all the tests using testConfRoundTrip useless since they always pass even if the functionality they're supposed to be testing give a different output. Since it's a real bug, I think it deserves a separate commit.
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virconftest.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/tests/virconftest.c b/tests/virconftest.c index ab29b5b712..c683344f49 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -32,40 +32,31 @@ static int testConfRoundTrip(const void *opaque) { const char *name = opaque; - int ret = -1; g_autoptr(virConf) conf = NULL; int len = 10000; - char *buffer = NULL; - char *srcfile = NULL; - char *dstfile = NULL; + g_autofree char *buffer = NULL; + g_autofree char *srcfile = NULL; + g_autofree char *dstfile = NULL;
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) { - fprintf(stderr, "out of memory\n"); - goto cleanup; - } + buffer = g_new0(char, len); conf = virConfReadFile(srcfile, 0); if (conf == NULL) { fprintf(stderr, "Failed to process %s\n", srcfile); - goto cleanup; + return -1; }
- ret = virConfWriteMem(buffer, &len, conf); - if (ret < 0) { + + if (virConfWriteMem(buffer, &len, conf) < 0) {
With this change separated. Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
fprintf(stderr, "Failed to serialize %s back\n", srcfile); - goto cleanup; + return -1; }
if (virTestCompareToFile(buffer, dstfile) < 0) - goto cleanup; + return -1;
- ret = 0; - cleanup: - VIR_FREE(srcfile); - VIR_FREE(dstfile); - VIR_FREE(buffer); - return ret; + return 0; }
-- 2.26.2
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virdevmapper.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c index 7c89c2a952..4d27c9f104 100644 --- a/src/util/virdevmapper.c +++ b/src/util/virdevmapper.c @@ -260,9 +260,7 @@ virDevMapperGetTargetsImpl(int controlFD, return -1; } - if (VIR_ALLOC_N_QUIET(devPaths, deps->count + 1) < 0) - return -1; - + devPaths = g_new0(char *, deps->count + 1); for (i = 0; i < deps->count; i++) { devPaths[i] = g_strdup_printf("/dev/block/%u:%u", major(deps->dev[i]), -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virdevmapper.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/util/virjson.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 6921eccb60..6511b4e641 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1243,8 +1243,8 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val, if (val->type != VIR_JSON_TYPE_ARRAY) return -1; - if (VIR_ALLOC_N_QUIET(elems, val->data.array.nvalues) < 0) - return -1; + /* elems might be NULL if val->data.array.nvalues is 0 */ + elems = g_new0(unsigned long long, val->data.array.nvalues); /* first pass converts array members to numbers and finds the maximum */ for (i = 0; i < val->data.array.nvalues; i++) { -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virjson.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/virjson.c b/src/util/virjson.c index 6921eccb60..6511b4e641 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1243,8 +1243,8 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val, if (val->type != VIR_JSON_TYPE_ARRAY) return -1;
- if (VIR_ALLOC_N_QUIET(elems, val->data.array.nvalues) < 0) - return -1; + /* elems might be NULL if val->data.array.nvalues is 0 */
Is this worthy of commenting? Even the calloc man page says this can happen.
+ elems = g_new0(unsigned long long, val->data.array.nvalues);
/* first pass converts array members to numbers and finds the maximum */ for (i = 0; i < val->data.array.nvalues; i++) { -- 2.26.2
Without the comment: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 15 --------------- 1 file changed, 15 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 -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 15 --------------- 1 file changed, 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 0ed132a829..6efc6102bd 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)); + masterKey = g_renew(uint8_t, masterKey, masterKeyLen); priv->masterKey = masterKey; priv->masterKeyLen = masterKeyLen; -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6efc6102bd..785cee6f18 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -6357,7 +6357,7 @@ ssize_t qemuDomainLogContextRead(qemuDomainLogContextPtr ctxt, buf[got] = '\0'; - ignore_value(VIR_REALLOC_N_QUIET(buf, got + 1)); + buf = g_renew(char, buf, got + 1); buflen = got; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index dd60fb0ddf..b1af35b933 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)); + buf = g_renew(char, buf, got + 1); *msg = buf; return 0; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virpcimock.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 6b1f2f2a5a..c8aa8f3f01 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -305,11 +305,7 @@ add_fd(int fd, const char *path) fd, path, cb.fd, cb.path); } - if (VIR_REALLOC_N_QUIET(callbacks, nCallbacks + 1) < 0) { - errno = ENOMEM; - return -1; - } - + callbacks = g_renew(struct fdCallback, callbacks, nCallbacks + 1); callbacks[nCallbacks].path = g_strdup(path); callbacks[nCallbacks++].fd = fd; -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virpcimock.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virpcimock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index c8aa8f3f01..787172d24c 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -701,12 +701,12 @@ 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) - ABORT_OOM(); - + driver->vendor = g_renew(int, driver->vendor, driver->len + 1); driver->vendor[driver->len] = vendor; + + driver->device = g_renew(int, driver->device, driver->len + 1); driver->device[driver->len] = device; + driver->len++; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virpcimock.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tools/vsh.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/vsh.c b/tools/vsh.c index 0e8edcd78c..e3c2404a74 100644 --- a/tools/vsh.c +++ b/tools/vsh.c @@ -2709,13 +2709,11 @@ vshCompleterFilter(char ***list, return -1; list_len = virStringListLength((const char **) *list); - - if (VIR_ALLOC_N(newList, list_len + 1) < 0) - return -1; + newList = g_new0(char *, list_len + 1); for (i = 0; i < list_len; i++) { if (!STRPREFIX((*list)[i], text)) { - VIR_FREE((*list)[i]); + g_clear_pointer(&(*list)[i], g_free); continue; } @@ -2723,8 +2721,8 @@ vshCompleterFilter(char ***list, newList_len++; } - ignore_value(VIR_REALLOC_N_QUIET(newList, newList_len + 1)); - VIR_FREE(*list); + newList = g_renew(char *, newList, newList_len + 1); + g_free(*list); *list = newList; return 0; } -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tools/vsh.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virfile.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/util/virfile.c b/src/util/virfile.c index 90156845df..61d2c16072 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -4327,8 +4327,7 @@ virFileGetXAttrQuiet(const char *path, const char *name, char **value) { - char *buf = NULL; - int ret = -1; + g_autofree char *buf = NULL; /* We might be racing with somebody who sets the same attribute. */ while (1) { @@ -4337,15 +4336,14 @@ virFileGetXAttrQuiet(const char *path, /* The first call determines how many bytes we need to allocate. */ if ((need = getxattr(path, name, NULL, 0)) < 0) - goto cleanup; + return -1; - if (VIR_REALLOC_N_QUIET(buf, need + 1) < 0) - goto cleanup; + buf = g_renew(char, buf, need + 1); if ((got = getxattr(path, name, buf, need)) < 0) { if (errno == ERANGE) continue; - goto cleanup; + return -1; } buf[got] = '\0'; @@ -4353,10 +4351,7 @@ virFileGetXAttrQuiet(const char *path, } *value = g_steal_pointer(&buf); - ret = 0; - cleanup: - VIR_FREE(buf); - return ret; + return 0; } /** -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virfile.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 15 --------------- 1 file changed, 15 deletions(-) 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 -- 2.26.2
On a Friday in 2020, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/viralloc.h | 15 --------------- 1 file changed, 15 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On a Friday in 2020, Tim Wiederhake wrote:
See also https://www.redhat.com/archives/libvir-list/2020-September/msg00540.html and https://www.redhat.com/archives/libvir-list/2020-September/msg00610.html
Tim Wiederhake (21): util: Use glib memory functions in virErrorCopyNew util: Use glib memory functions in virLastErrorObject util: Remove VIR_ALLOC_QUIET tests: Use glib memory function in testConfRoundTrip util: Use glib memory functions in virJSONValueGetArrayAsBitmap util: Remove VIR_ALLOC_N_QUIET
I have pushed all the paches except these ^^^
qemu: Use glib memory functions in qemuDomainMasterKeyReadFile qemu: Use glib memory functions in qemuDomainLogContextRead qemu: Use glib memory functions in qemuProcessReadLog
These could have been squashed into one commit: qemu: remove use of VIR_REALLOC_N_QUIET Jano
src/qemu/qemu_domain.c | 4 ++-- src/qemu/qemu_process.c | 2 +- src/rpc/virnetmessage.c | 4 ++-- src/util/viralloc.h | 44 ----------------------------------------- src/util/virbitmap.c | 10 ++-------- src/util/virdevmapper.c | 4 +--- src/util/virerror.c | 30 ++++++++++++++-------------- src/util/virfile.c | 15 +++++--------- src/util/virjson.c | 4 ++-- src/util/virlog.c | 24 ++++++++-------------- src/util/virthread.c | 5 +---- tests/virconftest.c | 29 ++++++++++----------------- tests/virpcimock.c | 24 ++++++++-------------- tools/vsh.c | 10 ++++------ 14 files changed, 61 insertions(+), 148 deletions(-)
-- 2.26.2
participants (2)
-
Ján Tomko -
Tim Wiederhake