[libvirt PATCH v2 0/7] Remove VIR_{ALLOC, ALLOC_N, REALLOC_N}_QUIET macros.

V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00633.html Tim Wiederhake (7): tests: Fix false positive in testConfRoundTrip. 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 src/util/viralloc.h | 29 ----------------------------- src/util/virerror.c | 15 ++++----------- src/util/virjson.c | 3 +-- tests/virconftest.c | 28 +++++++++------------------- 4 files changed, 14 insertions(+), 61 deletions(-) -- 2.26.2

testConfRoundTrip would return 0 (success) if virConfWriteMem returned 0 (nothing written) and virTestCompareToFile failed. Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virconftest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/virconftest.c b/tests/virconftest.c index ab29b5b712..cac3718495 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -51,8 +51,7 @@ static int testConfRoundTrip(const void *opaque) fprintf(stderr, "Failed to process %s\n", srcfile); goto cleanup; } - 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; } -- 2.26.2

On a Monday in 2020, Tim Wiederhake wrote:
testConfRoundTrip would return 0 (success) if virConfWriteMem returned 0 (nothing written) and virTestCompareToFile failed.
s/returned 0/succeeded/ The caller treats all non-negative values as a success. (as is usual in the codebase) Jano
Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virconftest.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/tests/virconftest.c b/tests/virconftest.c index ab29b5b712..cac3718495 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -51,8 +51,7 @@ static int testConfRoundTrip(const void *opaque) fprintf(stderr, "Failed to process %s\n", srcfile); goto cleanup; } - 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; } -- 2.26.2

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index 6b057057a3..d89948f198 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -223,14 +223,8 @@ virCopyError(virErrorPtr from, virErrorPtr virErrorCopyNew(virErrorPtr err) { - virErrorPtr ret; - - if (VIR_ALLOC_QUIET(ret) < 0) - return NULL; - - if (virCopyError(err, ret) < 0) - VIR_FREE(ret); - + virErrorPtr ret = g_new0(virError, 1); + virCopyError(err, ret); return ret; } -- 2.26.2

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virerror.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/util/virerror.c b/src/util/virerror.c index d89948f198..80a7cfe0ed 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -235,10 +235,9 @@ virLastErrorObject(void) virErrorPtr err; err = virThreadLocalGet(&virLastErr); if (!err) { - if (VIR_ALLOC_QUIET(err) < 0) - return NULL; + err = g_new0(virError, 1); if (virThreadLocalSet(&virLastErr, err) < 0) - VIR_FREE(err); + g_clear_pointer(&err, g_free); } return err; } -- 2.26.2

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Ján Tomko <jtomko@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 3e57d8a603..fa2bc8a2bc 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

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- tests/virconftest.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/tests/virconftest.c b/tests/virconftest.c index cac3718495..8269730662 100644 --- a/tests/virconftest.c +++ b/tests/virconftest.c @@ -32,39 +32,30 @@ 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; } 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

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> --- src/util/virjson.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/util/virjson.c b/src/util/virjson.c index 6921eccb60..ba43d6b667 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1243,8 +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) - return -1; + 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

Signed-off-by: Tim Wiederhake <twiederh@redhat.com> Reviewed-by: Ján Tomko <jtomko@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 fa2bc8a2bc..a50cd5d632 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 Monday in 2020, Tim Wiederhake wrote:
V1: https://www.redhat.com/archives/libvir-list/2020-September/msg00633.html
Tim Wiederhake (7): tests: Fix false positive in testConfRoundTrip. 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
src/util/viralloc.h | 29 ----------------------------- src/util/virerror.c | 15 ++++----------- src/util/virjson.c | 3 +-- tests/virconftest.c | 28 +++++++++------------------- 4 files changed, 14 insertions(+), 61 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko
-
Tim Wiederhake