[PATCH 00/15] virbitmap: Refactor bitmap allocation
Peter Krempa (15): virQEMUCapsFlagsString: Remove unused function virBitmapToString: Remove unused 'prefix' and 'trim' arguments virBitmapToString|virBitmapNewString: Clarify semantics of the 'string' virBitmapToString: Properly handle empty bitmaps virbitmaptest: test13: Refactor memory cleanup virbitmaptest: Use g_auto(free) for cleanup virbitmaptest: Remove unnecessary error/cleanup labels util: virbitmap: Don't forbid 0 size bitmap virbitmaptest: Add few more cases for virBitmapToString virBitmapNewQuiet: Don't fail on unlikely overflow scenario util: bitmap: Remove virBitmapNewQuiet virBitmapNew: Don't force return value check util: bitmp: Remove virBitmapNewEmpty qemuSlirpNew: Use g_new0 to allocate the slirp object virBitmapNew: Don't check return value src/conf/capabilities.c | 7 +- src/conf/checkpoint_conf.c | 3 +- src/conf/domain_addr.c | 6 +- src/conf/domain_conf.c | 12 +- src/conf/node_device_conf.c | 6 +- src/conf/numa_conf.c | 2 +- src/conf/storage_conf.c | 3 +- src/conf/virnetworkobj.c | 3 +- src/libvirt_private.syms | 2 - src/libxl/libxl_capabilities.c | 4 - src/libxl/libxl_driver.c | 6 +- src/lxc/lxc_controller.c | 3 - src/qemu/qemu_capabilities.c | 9 +- src/qemu/qemu_capabilities.h | 2 - src/qemu/qemu_conf.c | 3 +- src/qemu/qemu_domain_address.c | 5 +- src/qemu/qemu_driver.c | 11 +- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_migration_cookie.c | 5 +- src/qemu/qemu_migration_params.c | 12 +- src/qemu/qemu_monitor.c | 3 +- src/qemu/qemu_namespace.c | 5 +- src/qemu/qemu_process.c | 5 +- src/qemu/qemu_slirp.c | 7 +- src/qemu/qemu_snapshot.c | 3 +- src/test/test_driver.c | 12 +- src/util/virbitmap.c | 102 +++----- src/util/virbitmap.h | 6 +- src/util/vircommand.c | 3 +- src/util/virdnsmasq.c | 7 +- src/util/virhostcpu.c | 14 +- src/util/virjson.c | 3 +- src/util/virnetdev.c | 3 +- src/util/virnuma.c | 8 +- src/util/virportallocator.c | 6 +- src/util/virprocess.c | 6 +- src/util/virresctrl.c | 12 +- src/util/virstoragefile.c | 3 +- src/util/virtpm.c | 2 +- src/vmx/vmx.c | 2 - src/vz/vz_sdk.c | 3 +- tests/qemumonitorjsontest.c | 3 - tests/testutils.c | 11 +- tests/virbitmaptest.c | 410 +++++++++++++----------------- tools/virsh-domain.c | 9 +- tools/virt-host-validate-common.c | 4 +- 46 files changed, 271 insertions(+), 488 deletions(-) -- 2.26.2
Unused since a7424faff0fe8270c18d5e9e3befcfd2822d421f Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ------ src/qemu/qemu_capabilities.h | 2 -- 2 files changed, 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 5dcfcd574d..ea2e4c0948 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2035,12 +2035,6 @@ virQEMUCapsClear(virQEMUCapsPtr qemuCaps, } -char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps) -{ - return virBitmapToString(qemuCaps->flags, true, false); -} - - bool virQEMUCapsGet(virQEMUCapsPtr qemuCaps, virQEMUCapsFlags flag) diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 98d70cfa0e..107056ba17 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -611,8 +611,6 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, bool virQEMUCapsSupportsVmport(virQEMUCapsPtr qemuCaps, const virDomainDef *def); -char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps); - const char *virQEMUCapsGetBinary(virQEMUCapsPtr qemuCaps); virArch virQEMUCapsGetArch(virQEMUCapsPtr qemuCaps); unsigned int virQEMUCapsGetVersion(virQEMUCapsPtr qemuCaps); -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Unused since a7424faff0fe8270c18d5e9e3befcfd2822d421f
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 6 ------ src/qemu/qemu_capabilities.h | 2 -- 2 files changed, 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
There's only one combination used so we can remove the rest. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 15 +-------------- src/util/virbitmap.h | 2 +- src/util/virresctrl.c | 2 +- tests/virbitmaptest.c | 2 +- 4 files changed, 4 insertions(+), 17 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 639103e518..ad5213f216 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -327,17 +327,13 @@ virBitmapGetBit(virBitmapPtr bitmap, /** * virBitmapToString: * @bitmap: Pointer to bitmap - * @prefix: Whether to prepend "0x" - * @trim: Whether to output only the minimum required characters * * Convert @bitmap to printable string. * * Returns pointer to the string or NULL on error. */ char * -virBitmapToString(virBitmapPtr bitmap, - bool prefix, - bool trim) +virBitmapToString(virBitmapPtr bitmap) { g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER; size_t sz; @@ -345,9 +341,6 @@ virBitmapToString(virBitmapPtr bitmap, size_t diff; char *ret = NULL; - if (prefix) - virBufferAddLit(&buf, "0x"); - sz = bitmap->map_len; while (sz--) { @@ -360,15 +353,9 @@ virBitmapToString(virBitmapPtr bitmap, if (!ret) return NULL; - if (!trim) - return ret; - if (bitmap->nbits != bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT) { char *tmp = ret; - if (prefix) - tmp += 2; - len = strlen(tmp); sz = VIR_DIV_UP(bitmap->nbits, 4); diff = len - sz; diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 7f1a109c01..615ec6e4af 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -81,7 +81,7 @@ virBitmapPtr virBitmapNewString(const char *string) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; -char *virBitmapToString(virBitmapPtr bitmap, bool prefix, bool trim) +char *virBitmapToString(virBitmapPtr bitmap) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; char *virBitmapFormat(virBitmapPtr bitmap); diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 6de700a645..2d945ff934 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1592,7 +1592,7 @@ virResctrlAllocFormatCache(virResctrlAllocPtr alloc, if (!mask) continue; - mask_str = virBitmapToString(mask, false, true); + mask_str = virBitmapToString(mask); if (!mask_str) return -1; diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 1c7dc1d610..e1a49bfe35 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -681,7 +681,7 @@ test13(const void *opaque G_GNUC_UNUSED) if (!map) goto cleanup; - str = virBitmapToString(map, false, true); + str = virBitmapToString(map); if (!str) goto cleanup; -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
There's only one combination used so we can remove the rest.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 15 +-------------- src/util/virbitmap.h | 2 +- src/util/virresctrl.c | 2 +- tests/virbitmaptest.c | 2 +- 4 files changed, 4 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string. * * Returns pointer to the string or NULL on error. */ @@ -1117,10 +1119,13 @@ virBitmapCountBits(virBitmapPtr bitmap) * virBitmapNewString: * @string: the string to be converted to a bitmap * - * Allocate a bitmap from a string of hexadecimal data. + * Allocate a bitmap and populate it from a string of hexadecimal data. Note + * that leftmost character in the string will correspond to the highest + * index/position in the bitmap. The size of the returned bitmap corresponds to + * 4 * the length of @string. * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap or NULL and reports an error if + * @string can't be converted. */ virBitmapPtr virBitmapNewString(const char *string) -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string.
the bits ... are considered or the bit ... is considered would mentioning that it is printed at the leftmost position be clearer?
* * Returns pointer to the string or NULL on error. */ @@ -1117,10 +1119,13 @@ virBitmapCountBits(virBitmapPtr bitmap) * virBitmapNewString: * @string: the string to be converted to a bitmap * - * Allocate a bitmap from a string of hexadecimal data. + * Allocate a bitmap and populate it from a string of hexadecimal data. Note + * that leftmost character in the string will correspond to the highest + * index/position in the bitmap. The size of the returned bitmap corresponds to + * 4 * the length of @string. * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap or NULL and reports an error if + * @string can't be converted. */ virBitmapPtr virBitmapNewString(const char *string)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string.
the bits ... are considered or the bit ... is considered
oops, I've rewrote it halfway through ...
would mentioning that it is printed at the leftmost position be clearer?
Well, the thing is that the leftmost digit in the output string represents more than one bit since it's hex. I thought about some wordign but couldn't come up with anything more appropriate. We could do: 'is considered as the most significant bit of the number represented by the output string', or just 'most significant bit of the output number". That way the reader knows it's a number and the semantics of the bit are then implicit.
On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string.
the bits ... are considered or the bit ... is considered
oops, I've rewrote it halfway through ...
would mentioning that it is printed at the leftmost position be clearer?
Well, the thing is that the leftmost digit in the output string represents more than one bit since it's hex. I thought about some wordign but couldn't come up with anything more appropriate.
We could do: 'is considered as the most significant bit of the number represented by the output string', or just 'most significant bit of the output number". That way the reader knows it's a number and the semantics of the bit are then implicit.
Either of those LGTM Jano
On Fri, Oct 02, 2020 at 17:44:13 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string.
the bits ... are considered or the bit ... is considered
oops, I've rewrote it halfway through ...
would mentioning that it is printed at the leftmost position be clearer?
Well, the thing is that the leftmost digit in the output string represents more than one bit since it's hex. I thought about some wordign but couldn't come up with anything more appropriate.
We could do: 'is considered as the most significant bit of the number represented by the output string', or just 'most significant bit of the output number". That way the reader knows it's a number and the semantics of the bit are then implicit.
Either of those LGTM
I'll go with: diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..ed28427736 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to a number where the bit with highest position/index in + * @bitmap represents the most significant bit and return the number in form + * of a hexadecimal string. * * Returns pointer to the string or NULL on error. */ @@ -1117,10 +1119,14 @@ virBitmapCountBits(virBitmapPtr bitmap) * virBitmapNewString: * @string: the string to be converted to a bitmap * - * Allocate a bitmap from a string of hexadecimal data. + * Allocate a bitmap and populate it from @string representing a number in + * hexadecimal format. Note that the most significant bit of the number + * represented by @string will correspond to the highest index/position in the + * bitmap. The size of the returned bitmap corresponds to 4 * the length of + * @string. * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap or NULL and reports an error if + * @string can't be converted. */ virBitmapPtr virBitmapNewString(const char *string)
On a Monday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 17:44:13 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string.
the bits ... are considered or the bit ... is considered
oops, I've rewrote it halfway through ...
would mentioning that it is printed at the leftmost position be clearer?
Well, the thing is that the leftmost digit in the output string represents more than one bit since it's hex. I thought about some wordign but couldn't come up with anything more appropriate.
We could do: 'is considered as the most significant bit of the number represented by the output string', or just 'most significant bit of the output number". That way the reader knows it's a number and the semantics of the bit are then implicit.
Either of those LGTM
I'll go with:
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..ed28427736 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to a number where the bit with highest position/index in + * @bitmap represents the most significant bit and return the number in form + * of a hexadecimal string. * * Returns pointer to the string or NULL on error. */ @@ -1117,10 +1119,14 @@ virBitmapCountBits(virBitmapPtr bitmap) * virBitmapNewString: * @string: the string to be converted to a bitmap * - * Allocate a bitmap from a string of hexadecimal data. + * Allocate a bitmap and populate it from @string representing a number in + * hexadecimal format. Note that the most significant bit of the number + * represented by @string will correspond to the highest index/position in the + * bitmap. The size of the returned bitmap corresponds to 4 * the length of + * @string. * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap or NULL and reports an error if + * @string can't be converted. */ virBitmapPtr virBitmapNewString(const char *string)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
virBitmapNewEmpty() can create a bitmap with 0 lenght. With such a bitmap virBitmapToString will return NULL rather than an empty string. Initialize the buffer to avoid that. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index fcb8e1101a..f1443c3ecf 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -345,6 +345,9 @@ virBitmapToString(virBitmapPtr bitmap) sz = bitmap->map_len; + /* initialize buffer to return empty string for 0 length bitmap */ + virBufferAdd(&buf, "", -1); + while (sz--) { virBufferAsprintf(&buf, "%0*lx", VIR_BITMAP_BITS_PER_UNIT / 4, -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
virBitmapNewEmpty() can create a bitmap with 0 lenght. With such a
*length
bitmap virBitmapToString will return NULL rather than an empty string. Initialize the buffer to avoid that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Move scope of variables and get rid of the 'cleanup' section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index e1a49bfe35..1578cd0612 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -670,37 +670,27 @@ test12(const void *opaque G_GNUC_UNUSED) static int test13(const void *opaque G_GNUC_UNUSED) { - virBitmapPtr map = NULL; const char *strings[] = { "1234feebee", "000c0fefe" }; - char *str = NULL; size_t i = 0; - int ret = -1; for (i = 0; i < G_N_ELEMENTS(strings); i++) { - map = virBitmapNewString(strings[i]); - if (!map) - goto cleanup; + g_autoptr(virBitmap) map = NULL; + g_autofree char *str = NULL; - str = virBitmapToString(map); - if (!str) - goto cleanup; + if (!(map = virBitmapNewString(strings[i]))) + return -1; + + if (!(str = virBitmapToString(map))) + return -1; if (STRNEQ(strings[i], str)) { - fprintf(stderr, "\n expected bitmap string '%s' actual string " - "'%s'\n", strings[i], str); - goto cleanup; + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + strings[i], str); + return -1; } - - VIR_FREE(str); - virBitmapFree(map); - map = NULL; } - ret = 0; - cleanup: - VIR_FREE(str); - virBitmapFree(map); - return ret; + return 0; } #undef TEST_MAP -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Move scope of variables and get rid of the 'cleanup' section.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index e1a49bfe35..1578cd0612 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -670,37 +670,27 @@ test12(const void *opaque G_GNUC_UNUSED) static int test13(const void *opaque G_GNUC_UNUSED) { - virBitmapPtr map = NULL; const char *strings[] = { "1234feebee", "000c0fefe" }; - char *str = NULL; size_t i = 0; - int ret = -1;
for (i = 0; i < G_N_ELEMENTS(strings); i++) { - map = virBitmapNewString(strings[i]); - if (!map) - goto cleanup; + g_autoptr(virBitmap) map = NULL; + g_autofree char *str = NULL;
- str = virBitmapToString(map); - if (!str) - goto cleanup; + if (!(map = virBitmapNewString(strings[i]))) + return -1; + + if (!(str = virBitmapToString(map))) + return -1;
if (STRNEQ(strings[i], str)) { - fprintf(stderr, "\n expected bitmap string '%s' actual string " - "'%s'\n", strings[i], str); - goto cleanup; + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + strings[i], str);
You also altered the error message.
+ return -1; } -
With that at least mentioned in the commit message: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 73 ++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 49 deletions(-) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 1578cd0612..bfca208a7f 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -29,7 +29,7 @@ static int test1(const void *data G_GNUC_UNUSED) { - virBitmapPtr bitmap; + g_autoptr(virBitmap) bitmap = NULL; int size; int bit; bool result; @@ -58,7 +58,6 @@ test1(const void *data G_GNUC_UNUSED) ret = 0; error: - virBitmapFree(bitmap); return ret; } @@ -85,8 +84,8 @@ static int test2(const void *data G_GNUC_UNUSED) { const char *bitsString1 = "1-32,50,88-99,1021-1023"; - char *bitsString2 = NULL; - virBitmapPtr bitmap = NULL; + g_autofree char *bitsString2 = NULL; + g_autoptr(virBitmap) bitmap = NULL; int ret = -1; int size = 1025; @@ -139,15 +138,13 @@ test2(const void *data G_GNUC_UNUSED) ret = 0; error: - virBitmapFree(bitmap); - VIR_FREE(bitsString2); return ret; } static int test3(const void *data G_GNUC_UNUSED) { - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL; int ret = -1; int size = 5; size_t i; @@ -167,7 +164,6 @@ test3(const void *data G_GNUC_UNUSED) ret = 0; error: - virBitmapFree(bitmap); return ret; } @@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED) 1, 5, 11, 13, 19, 21, 23, 24, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39 }; - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL; ssize_t i, j; if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size) @@ -285,11 +281,9 @@ test4(const void *data G_GNUC_UNUSED) if (virBitmapNextClearBit(bitmap, -1) != -1) goto error; - virBitmapFree(bitmap); return 0; error: - virBitmapFree(bitmap); return -1; } @@ -298,14 +292,14 @@ static int test5(const void *v G_GNUC_UNUSED) { char data[] = {0x01, 0x02, 0x00, 0x00, 0x04}; - unsigned char *data2 = NULL; + g_autofree unsigned char *data2 = NULL; int len2; int bits[] = {0, 9, 34}; - virBitmapPtr bitmap; + g_autoptr(virBitmap) bitmap = NULL; size_t i; ssize_t j; int ret = -1; - char *str = NULL; + g_autofree char *str = NULL; bitmap = virBitmapNewData(data, sizeof(data)); if (!bitmap) @@ -347,9 +341,6 @@ test5(const void *v G_GNUC_UNUSED) ret = 0; error: - VIR_FREE(str); - virBitmapFree(bitmap); - VIR_FREE(data2); return ret; } @@ -358,8 +349,8 @@ test5(const void *v G_GNUC_UNUSED) static int test6(const void *v G_GNUC_UNUSED) { - virBitmapPtr bitmap = NULL; - char *str = NULL; + g_autoptr(virBitmap) bitmap = NULL; + g_autofree char *str = NULL; int size = 64; int ret = -1; @@ -432,15 +423,12 @@ test6(const void *v G_GNUC_UNUSED) ret = 0; error: - virBitmapFree(bitmap); - VIR_FREE(str); return ret; } static int test7(const void *v G_GNUC_UNUSED) { - virBitmapPtr bitmap; size_t i; size_t maxBit[] = { 1, 8, 31, 32, 63, 64, 95, 96, 127, 128, 159, 160 @@ -448,7 +436,7 @@ test7(const void *v G_GNUC_UNUSED) size_t nmaxBit = 12; for (i = 0; i < nmaxBit; i++) { - bitmap = virBitmapNew(maxBit[i]); + g_autoptr(virBitmap) bitmap = virBitmapNew(maxBit[i]); if (!bitmap) goto error; @@ -466,21 +454,18 @@ test7(const void *v G_GNUC_UNUSED) virBitmapClearAll(bitmap); if (!virBitmapIsAllClear(bitmap)) goto error; - - virBitmapFree(bitmap); } return 0; error: - virBitmapFree(bitmap); return -1; } static int test8(const void *v G_GNUC_UNUSED) { - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL; char data[108] = {0x00,}; int ret = -1; @@ -499,7 +484,6 @@ test8(const void *v G_GNUC_UNUSED) ret = 0; cleanup: - virBitmapFree(bitmap); return ret; } @@ -509,7 +493,7 @@ static int test9(const void *opaque G_GNUC_UNUSED) { int ret = -1; - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL; if (virBitmapParse("100000000", &bitmap, 20) != -1) goto cleanup; @@ -531,7 +515,6 @@ test9(const void *opaque G_GNUC_UNUSED) ret = 0; cleanup: - virBitmapFree(bitmap); return ret; } @@ -540,7 +523,10 @@ static int test10(const void *opaque G_GNUC_UNUSED) { int ret = -1; - virBitmapPtr b1 = NULL, b2 = NULL, b3 = NULL, b4 = NULL; + g_autoptr(virBitmap) b1 = NULL; + g_autoptr(virBitmap) b2 = NULL; + g_autoptr(virBitmap) b3 = NULL; + g_autoptr(virBitmap) b4 = NULL; if (virBitmapParseSeparator("0-3,5-8,11-15f16", 'f', &b1, 20) < 0 || virBitmapParse("4,9,10,16-19", &b2, 20) < 0 || @@ -561,10 +547,6 @@ test10(const void *opaque G_GNUC_UNUSED) ret = 0; cleanup: - virBitmapFree(b1); - virBitmapFree(b2); - virBitmapFree(b3); - virBitmapFree(b4); return ret; } @@ -578,9 +560,9 @@ static int test11(const void *opaque) { const struct testBinaryOpData *data = opaque; - virBitmapPtr amap = NULL; - virBitmapPtr bmap = NULL; - virBitmapPtr resmap = NULL; + g_autoptr(virBitmap) amap = NULL; + g_autoptr(virBitmap) bmap = NULL; + g_autoptr(virBitmap) resmap = NULL; int ret = -1; if (virBitmapParse(data->a, &amap, 256) < 0 || @@ -600,9 +582,6 @@ test11(const void *opaque) ret = 0; cleanup: - virBitmapFree(amap); - virBitmapFree(bmap); - virBitmapFree(resmap); return ret; } @@ -631,7 +610,7 @@ test11(const void *opaque) static int test12(const void *opaque G_GNUC_UNUSED) { - virBitmapPtr map = virBitmapNewEmpty(); + g_autoptr(virBitmap) map = virBitmapNewEmpty(); int ret = -1; TEST_MAP(0, ""); @@ -661,7 +640,6 @@ test12(const void *opaque G_GNUC_UNUSED) ret = 0; cleanup: - virBitmapFree(map); return ret; } @@ -699,9 +677,9 @@ static int test14(const void *opaque) { const struct testBinaryOpData *data = opaque; - virBitmapPtr amap = NULL; - virBitmapPtr bmap = NULL; - virBitmapPtr resmap = NULL; + g_autoptr(virBitmap) amap = NULL; + g_autoptr(virBitmap) bmap = NULL; + g_autoptr(virBitmap) resmap = NULL; int ret = -1; if (virBitmapParse(data->a, &amap, 256) < 0 || @@ -721,9 +699,6 @@ test14(const void *opaque) ret = 0; cleanup: - virBitmapFree(amap); - virBitmapFree(bmap); - virBitmapFree(resmap); return ret; } -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 73 ++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 1578cd0612..bfca208a7f 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED) 1, 5, 11, 13, 19, 21, 23, 24, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39 }; - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL;
Here, bitmap is also freed in the middle of the function (it seems these are three independent tests squashed into one function)
ssize_t i, j;
if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size) @@ -285,11 +281,9 @@ test4(const void *data G_GNUC_UNUSED) if (virBitmapNextClearBit(bitmap, -1) != -1) goto error;
- virBitmapFree(bitmap); return 0;
error: - virBitmapFree(bitmap); return -1; }
@@ -298,14 +292,14 @@ static int test5(const void *v G_GNUC_UNUSED) { char data[] = {0x01, 0x02, 0x00, 0x00, 0x04}; - unsigned char *data2 = NULL; + g_autofree unsigned char *data2 = NULL; int len2; int bits[] = {0, 9, 34}; - virBitmapPtr bitmap; + g_autoptr(virBitmap) bitmap = NULL; size_t i; ssize_t j; int ret = -1;
- char *str = NULL; + g_autofree char *str = NULL;
This one is also freed in the middle.
bitmap = virBitmapNewData(data, sizeof(data)); if (!bitmap) @@ -347,9 +341,6 @@ test5(const void *v G_GNUC_UNUSED)
ret = 0; error: - VIR_FREE(str); - virBitmapFree(bitmap); - VIR_FREE(data2); return ret; }
@@ -358,8 +349,8 @@ test5(const void *v G_GNUC_UNUSED) static int test6(const void *v G_GNUC_UNUSED) { - virBitmapPtr bitmap = NULL; - char *str = NULL; + g_autoptr(virBitmap) bitmap = NULL; + g_autofree char *str = NULL;
Same here.
int size = 64; int ret = -1;
@@ -631,7 +610,7 @@ test11(const void *opaque) static int test12(const void *opaque G_GNUC_UNUSED) { - virBitmapPtr map = virBitmapNewEmpty(); + g_autoptr(virBitmap) map = virBitmapNewEmpty();
`map` is freed in the middle of the function.
int ret = -1;
TEST_MAP(0, ""); @@ -661,7 +640,6 @@ test12(const void *opaque G_GNUC_UNUSED) ret = 0;
cleanup: - virBitmapFree(map); return ret; }
Jano
On Fri, Oct 02, 2020 at 10:50:55 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 73 ++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 1578cd0612..bfca208a7f 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED) 1, 5, 11, 13, 19, 21, 23, 24, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39 }; - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL;
Here, bitmap is also freed in the middle of the function (it seems these are three independent tests squashed into one function)
What's worse, test4 actually doesn't report any errors if any of the cases would fail. You'd have to use a debugger to figure out what's wrong. Note that all of the cases you mention below actually clear the pointer when the contents are freed, so this patch is still correctly handling the memory. Should I post another version where I add separate variables for any reuse?
On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 10:50:55 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 73 ++++++++++++++----------------------------- 1 file changed, 24 insertions(+), 49 deletions(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 1578cd0612..bfca208a7f 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -185,7 +181,7 @@ test4(const void *data G_GNUC_UNUSED) 1, 5, 11, 13, 19, 21, 23, 24, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39 }; - virBitmapPtr bitmap = NULL; + g_autoptr(virBitmap) bitmap = NULL;
Here, bitmap is also freed in the middle of the function (it seems these are three independent tests squashed into one function)
What's worse, test4 actually doesn't report any errors if any of the cases would fail. You'd have to use a debugger to figure out what's wrong.
Note that all of the cases you mention below actually clear the pointer when the contents are freed, so this patch is still correctly handling the memory.
Yes, I do not object to the current handling of the memory - just that mixing the autofree approach with manual freeing can possibly lead us to a change that will disrupt that balance. https://www.redhat.com/archives/libvir-list/2019-July/msg00987.html
Should I post another version where I add separate variables for any reuse?
I was thinking they could be separated into test4a test4b etc, but that works too. Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 264 +++++++++++++++++------------------------- 1 file changed, 108 insertions(+), 156 deletions(-) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index bfca208a7f..56110971c9 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -33,32 +33,28 @@ test1(const void *data G_GNUC_UNUSED) int size; int bit; bool result; - int ret = -1; size = 1024; bit = 100; if (!(bitmap = virBitmapNew(size))) - goto error; + return -1; if (virBitmapSetBit(bitmap, bit) < 0) - goto error; + return -1; if (virBitmapGetBit(bitmap, bit, &result) < 0) - goto error; + return -1; if (!result) - goto error; + return -1; if (virBitmapGetBit(bitmap, bit + 1, &result) < 0) - goto error; + return -1; if (result) - goto error; - - ret = 0; + return -1; - error: - return ret; + return 0; } static int @@ -86,85 +82,78 @@ test2(const void *data G_GNUC_UNUSED) const char *bitsString1 = "1-32,50,88-99,1021-1023"; g_autofree char *bitsString2 = NULL; g_autoptr(virBitmap) bitmap = NULL; - int ret = -1; int size = 1025; if (virBitmapParse(bitsString1, &bitmap, size) < 0) - goto error; + return -1; if (testBit(bitmap, 1, 32, true) < 0) - goto error; + return -1; if (testBit(bitmap, 50, 50, true) < 0) - goto error; + return -1; if (testBit(bitmap, 88, 99, true) < 0) - goto error; + return -1; if (testBit(bitmap, 1021, 1023, true) < 0) - goto error; + return -1; if (testBit(bitmap, 0, 0, false) < 0) - goto error; + return -1; if (testBit(bitmap, 33, 49, false) < 0) - goto error; + return -1; if (testBit(bitmap, 51, 87, false) < 0) - goto error; + return -1; if (testBit(bitmap, 100, 1020, false) < 0) - goto error; + return -1; if (virBitmapCountBits(bitmap) != 48) - goto error; + return -1; if (!(bitsString2 = virBitmapFormat(bitmap))) - goto error; + return -1; if (strcmp(bitsString1, bitsString2)) - goto error; + return -1; virBitmapSetAll(bitmap); if (testBit(bitmap, 0, size - 1, true) < 0) - goto error; + return -1; if (virBitmapCountBits(bitmap) != size) - goto error; + return -1; if (!virBitmapIsAllSet(bitmap)) - goto error; + return -1; virBitmapClearAll(bitmap); if (!virBitmapIsAllClear(bitmap)) - goto error; + return -1; if (testBit(bitmap, 0, size - 1, false) < 0) - goto error; + return -1; if (virBitmapCountBits(bitmap) != 0) - goto error; - - ret = 0; + return -1; - error: - return ret; + return 0; } static int test3(const void *data G_GNUC_UNUSED) { g_autoptr(virBitmap) bitmap = NULL; - int ret = -1; int size = 5; size_t i; if ((bitmap = virBitmapNew(size)) == NULL) - goto error; + return -1; for (i = 0; i < size; i++) ignore_value(virBitmapSetBit(bitmap, i)); if (!virBitmapIsAllSet(bitmap)) - goto error; + return -1; virBitmapClearAll(bitmap); if (!virBitmapIsAllClear(bitmap)) - goto error; - ret = 0; + return -1; - error: - return ret; + return 0; } /* test for virBitmapNextSetBit, virBitmapLastSetBit, virBitmapNextClearBit */ @@ -185,20 +174,20 @@ test4(const void *data G_GNUC_UNUSED) ssize_t i, j; if (G_N_ELEMENTS(bitsPos) + G_N_ELEMENTS(bitsPosInv) != size) - goto error; + return -1; /* 0. empty set */ bitmap = virBitmapNewEmpty(); if (virBitmapNextSetBit(bitmap, -1) != -1) - goto error; + return -1; if (virBitmapLastSetBit(bitmap) != -1) - goto error; + return -1; if (virBitmapNextClearBit(bitmap, -1) != -1) - goto error; + return -1; virBitmapFree(bitmap); bitmap = NULL; @@ -207,23 +196,23 @@ test4(const void *data G_GNUC_UNUSED) bitmap = virBitmapNew(size); if (!bitmap) - goto error; + return -1; if (virBitmapNextSetBit(bitmap, -1) != -1) - goto error; + return -1; if (virBitmapLastSetBit(bitmap) != -1) - goto error; + return -1; for (i = 0; i < size; i++) { if (virBitmapNextClearBit(bitmap, i - 1) != i) - goto error; + return -1; } if (virBitmapNextClearBit(bitmap, i) != -1) - goto error; + return -1; if (!virBitmapIsAllClear(bitmap)) - goto error; + return -1; virBitmapFree(bitmap); bitmap = NULL; @@ -231,9 +220,9 @@ test4(const void *data G_GNUC_UNUSED) /* 2. partial set */ if (virBitmapParse(bitsString, &bitmap, size) < 0) - goto error; + return -1; if (!bitmap) - goto error; + return -1; j = 0; i = -1; @@ -241,16 +230,16 @@ test4(const void *data G_GNUC_UNUSED) while (j < G_N_ELEMENTS(bitsPos)) { i = virBitmapNextSetBit(bitmap, i); if (i != bitsPos[j++]) - goto error; + return -1; } if (virBitmapNextSetBit(bitmap, i) != -1) - goto error; + return -1; j = sizeof(bitsPos)/sizeof(int) - 1; if (virBitmapLastSetBit(bitmap) != bitsPos[j]) - goto error; + return -1; j = 0; i = -1; @@ -258,11 +247,11 @@ test4(const void *data G_GNUC_UNUSED) while (j < G_N_ELEMENTS(bitsPosInv)) { i = virBitmapNextClearBit(bitmap, i); if (i != bitsPosInv[j++]) - goto error; + return -1; } if (virBitmapNextClearBit(bitmap, i) != -1) - goto error; + return -1; /* 3. full set */ @@ -270,21 +259,18 @@ test4(const void *data G_GNUC_UNUSED) for (i = 0; i < size; i++) { if (virBitmapNextSetBit(bitmap, i - 1) != i) - goto error; + return -1; } if (virBitmapNextSetBit(bitmap, i) != -1) - goto error; + return -1; if (virBitmapLastSetBit(bitmap) != size - 1) - goto error; + return -1; if (virBitmapNextClearBit(bitmap, -1) != -1) - goto error; + return -1; return 0; - - error: - return -1; } /* test for virBitmapNewData/ToData/DataFormat */ @@ -298,28 +284,27 @@ test5(const void *v G_GNUC_UNUSED) g_autoptr(virBitmap) bitmap = NULL; size_t i; ssize_t j; - int ret = -1; g_autofree char *str = NULL; bitmap = virBitmapNewData(data, sizeof(data)); if (!bitmap) - goto error; + return -1; i = 0; j = -1; while (i < sizeof(bits)/sizeof(int) && (j = virBitmapNextSetBit(bitmap, j)) >= 0) { if (j != bits[i++]) - goto error; + return -1; } if (virBitmapNextSetBit(bitmap, j) > 0) - goto error; + return -1; ignore_value(virBitmapSetBit(bitmap, 2)); ignore_value(virBitmapSetBit(bitmap, 15)); if (virBitmapToData(bitmap, &data2, &len2) < 0) - goto error; + return -1; if (len2 != sizeof(data) || data2[0] != 0x05 || @@ -327,21 +312,19 @@ test5(const void *v G_GNUC_UNUSED) data2[2] != 0x00 || data2[3] != 0x00 || data2[4] != 0x04) - goto error; + return -1; if (!(str = virBitmapDataFormat(data, sizeof(data)))) - goto error; + return -1; if (STRNEQ(str, "0,9,34")) - goto error; + return -1; VIR_FREE(str); if (!(str = virBitmapDataFormat(data2, len2))) - goto error; + return -1; if (STRNEQ(str, "0,2,9,15,34")) - goto error; + return -1; - ret = 0; - error: - return ret; + return 0; } @@ -352,28 +335,27 @@ test6(const void *v G_GNUC_UNUSED) g_autoptr(virBitmap) bitmap = NULL; g_autofree char *str = NULL; int size = 64; - int ret = -1; bitmap = virBitmapNew(size); if (!bitmap) - goto error; + return -1; str = virBitmapFormat(bitmap); if (!str) - goto error; + return -1; if (STRNEQ(str, "")) - goto error; + return -1; VIR_FREE(str); ignore_value(virBitmapSetBit(bitmap, 0)); str = virBitmapFormat(bitmap); if (!str) - goto error; + return -1; if (STRNEQ(str, "0")) - goto error; + return -1; VIR_FREE(str); @@ -381,20 +363,20 @@ test6(const void *v G_GNUC_UNUSED) ignore_value(virBitmapSetBit(bitmap, 5)); str = virBitmapFormat(bitmap); if (!str) - goto error; + return -1; if (STRNEQ(str, "0,4-5")) - goto error; + return -1; VIR_FREE(str); ignore_value(virBitmapSetBit(bitmap, 6)); str = virBitmapFormat(bitmap); if (!str) - goto error; + return -1; if (STRNEQ(str, "0,4-6")) - goto error; + return -1; VIR_FREE(str); @@ -404,10 +386,10 @@ test6(const void *v G_GNUC_UNUSED) ignore_value(virBitmapSetBit(bitmap, 16)); str = virBitmapFormat(bitmap); if (!str) - goto error; + return -1; if (STRNEQ(str, "0,4-6,13-16")) - goto error; + return -1; VIR_FREE(str); @@ -415,15 +397,12 @@ test6(const void *v G_GNUC_UNUSED) ignore_value(virBitmapSetBit(bitmap, 63)); str = virBitmapFormat(bitmap); if (!str) - goto error; + return -1; if (STRNEQ(str, "0,4-6,13-16,62-63")) - goto error; - + return -1; - ret = 0; - error: - return ret; + return 0; } static int @@ -438,28 +417,25 @@ test7(const void *v G_GNUC_UNUSED) for (i = 0; i < nmaxBit; i++) { g_autoptr(virBitmap) bitmap = virBitmapNew(maxBit[i]); if (!bitmap) - goto error; + return -1; if (virBitmapIsAllSet(bitmap)) - goto error; + return -1; ignore_value(virBitmapSetBit(bitmap, 1)); if (virBitmapIsAllSet(bitmap)) - goto error; + return -1; virBitmapSetAll(bitmap); if (!virBitmapIsAllSet(bitmap)) - goto error; + return -1; virBitmapClearAll(bitmap); if (!virBitmapIsAllClear(bitmap)) - goto error; + return -1; } return 0; - - error: - return -1; } static int @@ -467,24 +443,21 @@ test8(const void *v G_GNUC_UNUSED) { g_autoptr(virBitmap) bitmap = NULL; char data[108] = {0x00,}; - int ret = -1; bitmap = virBitmapNewData(data, sizeof(data)); if (!bitmap) - goto cleanup; + return -1; if (!virBitmapIsAllClear(bitmap)) - goto cleanup; + return -1; if (virBitmapSetBit(bitmap, 11) < 0) - goto cleanup; + return -1; if (virBitmapIsAllClear(bitmap)) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } @@ -492,37 +465,32 @@ test8(const void *v G_GNUC_UNUSED) static int test9(const void *opaque G_GNUC_UNUSED) { - int ret = -1; g_autoptr(virBitmap) bitmap = NULL; if (virBitmapParse("100000000", &bitmap, 20) != -1) - goto cleanup; + return -1; if (bitmap) - goto cleanup; + return -1; if (virBitmapParse("1-1000000000", &bitmap, 20) != -1) - goto cleanup; + return -1; if (bitmap) - goto cleanup; + return -1; if (virBitmapParse("1-10^10000000000", &bitmap, 20) != -1) - goto cleanup; + return -1; if (bitmap) - goto cleanup; - - ret = 0; - cleanup: - return ret; + return -1; + return 0; } static int test10(const void *opaque G_GNUC_UNUSED) { - int ret = -1; g_autoptr(virBitmap) b1 = NULL; g_autoptr(virBitmap) b2 = NULL; g_autoptr(virBitmap) b3 = NULL; @@ -532,10 +500,10 @@ test10(const void *opaque G_GNUC_UNUSED) virBitmapParse("4,9,10,16-19", &b2, 20) < 0 || virBitmapParse("15", &b3, 20) < 0 || virBitmapParse("0,^0", &b4, 20) < 0) - goto cleanup; + return -1; if (!virBitmapIsAllClear(b4)) - goto cleanup; + return -1; if (virBitmapOverlaps(b1, b2) || virBitmapOverlaps(b1, b4) || @@ -543,11 +511,9 @@ test10(const void *opaque G_GNUC_UNUSED) virBitmapOverlaps(b2, b4) || !virBitmapOverlaps(b1, b3) || virBitmapOverlaps(b3, b4)) - goto cleanup; + return -1; - ret = 0; - cleanup: - return ret; + return 0; } struct testBinaryOpData { @@ -563,12 +529,11 @@ test11(const void *opaque) g_autoptr(virBitmap) amap = NULL; g_autoptr(virBitmap) bmap = NULL; g_autoptr(virBitmap) resmap = NULL; - int ret = -1; if (virBitmapParse(data->a, &amap, 256) < 0 || virBitmapParse(data->b, &bmap, 256) < 0 || virBitmapParse(data->res, &resmap, 256) < 0) - goto cleanup; + return -1; virBitmapIntersect(amap, bmap); @@ -576,14 +541,10 @@ test11(const void *opaque) fprintf(stderr, "\n bitmap intersection failed: intersect('%s','%s') !='%s'\n", data->a, data->b, data->res); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - - return ret; + return 0; } #define TEST_MAP(sz, expect) \ @@ -592,7 +553,7 @@ test11(const void *opaque) if (virBitmapSize(map) != sz) { \ fprintf(stderr, "\n expected bitmap size: '%d' actual size: " \ "'%zu'\n", sz, virBitmapSize(map)); \ - goto cleanup; \ + return -1; \ } \ \ actual = virBitmapFormat(map); \ @@ -601,7 +562,7 @@ test11(const void *opaque) fprintf(stderr, "\n expected bitmap contents '%s' actual contents "\ "'%s'\n", NULLSTR(expect), NULLSTR(actual)); \ VIR_FREE(actual); \ - goto cleanup; \ + return -1; \ } \ VIR_FREE(actual); \ } while (0) @@ -611,23 +572,22 @@ static int test12(const void *opaque G_GNUC_UNUSED) { g_autoptr(virBitmap) map = virBitmapNewEmpty(); - int ret = -1; TEST_MAP(0, ""); if (virBitmapSetBitExpand(map, 128) < 0) - goto cleanup; + return -1; TEST_MAP(129, "128"); if (virBitmapClearBitExpand(map, 150) < 0) - goto cleanup; + return -1; TEST_MAP(151, "128"); virBitmapFree(map); if (!(map = virBitmapParseUnlimited("34,1023"))) - goto cleanup; + return -1; TEST_MAP(1024, "34,1023"); @@ -637,10 +597,7 @@ test12(const void *opaque G_GNUC_UNUSED) virBitmapShrink(map, 34); TEST_MAP(34, ""); - ret = 0; - - cleanup: - return ret; + return 0; } @@ -680,12 +637,11 @@ test14(const void *opaque) g_autoptr(virBitmap) amap = NULL; g_autoptr(virBitmap) bmap = NULL; g_autoptr(virBitmap) resmap = NULL; - int ret = -1; if (virBitmapParse(data->a, &amap, 256) < 0 || virBitmapParse(data->b, &bmap, 256) < 0 || virBitmapParse(data->res, &resmap, 256) < 0) - goto cleanup; + return -1; virBitmapSubtract(amap, bmap); @@ -693,14 +649,10 @@ test14(const void *opaque) fprintf(stderr, "\n bitmap subtraction failed: '%s' - '%s' != '%s'\n", data->a, data->b, data->res); - goto cleanup; + return -1; } - ret = 0; - - cleanup: - - return ret; + return 0; } /* virBitmapUnion() */ -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 264 +++++++++++++++++------------------------- 1 file changed, 108 insertions(+), 156 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We now have APIs which automatically expand the bitmap and also API which allocates a 0 size bitmap. Remove the condition from virBitmapNew. Effectively reverts ce49cfb48ad Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index f1443c3ecf..5df7ea7838 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -55,8 +55,8 @@ struct _virBitmap { * * Allocate a bitmap capable of containing @size bits. * - * Returns a pointer to the allocated bitmap or NULL if either memory cannot be - * allocated or size is 0. Does not report libvirt errors. + * Returns a pointer to the allocated bitmap or NULL if memory cannot be + * allocated. Does not report libvirt errors. */ virBitmapPtr virBitmapNewQuiet(size_t size) @@ -64,12 +64,16 @@ virBitmapNewQuiet(size_t size) virBitmapPtr bitmap; size_t sz; - if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size || size == 0) + if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size) return NULL; sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT); bitmap = g_new0(virBitmap, 1); + + if (size == 0) + return bitmap; + bitmap->map = g_new0(unsigned long, sz); bitmap->nbits = size; bitmap->map_len = sz; -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
We now have APIs which automatically expand the bitmap and also API which allocates a 0 size bitmap. Remove the condition from virBitmapNew.
Effectively reverts ce49cfb48ad
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Test an empty bitmap including it's extension via the self-expanding APIs and and a "0" and "" strings when converting the string back and forth. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 56110971c9..c14a6c7e26 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -605,7 +605,7 @@ test12(const void *opaque G_GNUC_UNUSED) static int test13(const void *opaque G_GNUC_UNUSED) { - const char *strings[] = { "1234feebee", "000c0fefe" }; + const char *strings[] = { "1234feebee", "000c0fefe", "0", "" }; size_t i = 0; for (i = 0; i < G_N_ELEMENTS(strings); i++) { @@ -684,6 +684,36 @@ test15(const void *opaque) } +/* virBitmapNewEmpty + virBitmapToString */ +static int +test16(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) map = virBitmapNewEmpty(); + g_autofree char *res_empty = NULL; + g_autofree char *res_set = NULL; + + if (!(res_empty = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_empty, "")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "", NULLSTR(res_empty)); + return -1; + } + + if (virBitmapSetBitExpand(map, 2) < 0 || + virBitmapSetBitExpand(map, 11) < 0) + abort(); + + if (!(res_set = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_set, "804")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "804", NULLSTR(res_set)); + return -1; + } + + return 0; +} + + #define TESTBINARYOP(A, B, RES, FUNC) \ testBinaryOpData.a = A; \ testBinaryOpData.b = B; \ @@ -751,6 +781,9 @@ mymain(void) TESTBINARYOP("12345", "0,^0", "12345", test15); TESTBINARYOP("0,^0", "0,^0", "0,^0", test15); + if (virTestRun("test16", test16, NULL) < 0) + ret = -1; + return ret; } -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Test an empty bitmap including it's extension via the self-expanding APIs and and a "0" and "" strings when converting the string back and forth.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 56110971c9..c14a6c7e26 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -605,7 +605,7 @@ test12(const void *opaque G_GNUC_UNUSED) static int test13(const void *opaque G_GNUC_UNUSED) { - const char *strings[] = { "1234feebee", "000c0fefe" }; + const char *strings[] = { "1234feebee", "000c0fefe", "0", "" }; size_t i = 0;
for (i = 0; i < G_N_ELEMENTS(strings); i++) { @@ -684,6 +684,36 @@ test15(const void *opaque) }
+/* virBitmapNewEmpty + virBitmapToString */ +static int +test16(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) map = virBitmapNewEmpty(); + g_autofree char *res_empty = NULL; + g_autofree char *res_set = NULL; + + if (!(res_empty = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_empty, "")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "", NULLSTR(res_empty)); + return -1; + } + + if (virBitmapSetBitExpand(map, 2) < 0 || + virBitmapSetBitExpand(map, 11) < 0) + abort();
While this should be dead code with the current virBitmap APIs, the point of the test is to check that they APIs behave correctly. We should report a proper error instead of aborting.
+ + if (!(res_set = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_set, "804")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "804", NULLSTR(res_set)); + return -1; + } + + return 0; +} + + #define TESTBINARYOP(A, B, RES, FUNC) \ testBinaryOpData.a = A; \ testBinaryOpData.b = B; \
With the error message added: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Fri, Oct 02, 2020 at 12:47:48 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Test an empty bitmap including it's extension via the self-expanding APIs and and a "0" and "" strings when converting the string back and forth.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 56110971c9..c14a6c7e26 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -605,7 +605,7 @@ test12(const void *opaque G_GNUC_UNUSED) static int test13(const void *opaque G_GNUC_UNUSED) { - const char *strings[] = { "1234feebee", "000c0fefe" }; + const char *strings[] = { "1234feebee", "000c0fefe", "0", "" }; size_t i = 0;
for (i = 0; i < G_N_ELEMENTS(strings); i++) { @@ -684,6 +684,36 @@ test15(const void *opaque) }
+/* virBitmapNewEmpty + virBitmapToString */ +static int +test16(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) map = virBitmapNewEmpty(); + g_autofree char *res_empty = NULL; + g_autofree char *res_set = NULL; + + if (!(res_empty = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_empty, "")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "", NULLSTR(res_empty)); + return -1; + } + + if (virBitmapSetBitExpand(map, 2) < 0 || + virBitmapSetBitExpand(map, 11) < 0) + abort();
While this should be dead code with the current virBitmap APIs, the point of the test is to check that they APIs behave correctly. We should report a proper error instead of aborting.
There are already tests which check proper extension of the bitmap. This is merely to extend it for testing that the bitmap is stringified correctly so I wouldn't consider this a part of the test.
+ + if (!(res_set = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_set, "804")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "804", NULLSTR(res_set)); + return -1; + } + + return 0; +} + + #define TESTBINARYOP(A, B, RES, FUNC) \ testBinaryOpData.a = A; \ testBinaryOpData.b = B; \
With the error message added: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 12:47:48 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Test an empty bitmap including it's extension via the self-expanding APIs and and a "0" and "" strings when converting the string back and forth.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/virbitmaptest.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 56110971c9..c14a6c7e26 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -605,7 +605,7 @@ test12(const void *opaque G_GNUC_UNUSED) static int test13(const void *opaque G_GNUC_UNUSED) { - const char *strings[] = { "1234feebee", "000c0fefe" }; + const char *strings[] = { "1234feebee", "000c0fefe", "0", "" }; size_t i = 0;
for (i = 0; i < G_N_ELEMENTS(strings); i++) { @@ -684,6 +684,36 @@ test15(const void *opaque) }
+/* virBitmapNewEmpty + virBitmapToString */ +static int +test16(const void *opaque G_GNUC_UNUSED) +{ + g_autoptr(virBitmap) map = virBitmapNewEmpty(); + g_autofree char *res_empty = NULL; + g_autofree char *res_set = NULL; + + if (!(res_empty = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_empty, "")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "", NULLSTR(res_empty)); + return -1; + } + + if (virBitmapSetBitExpand(map, 2) < 0 || + virBitmapSetBitExpand(map, 11) < 0) + abort();
While this should be dead code with the current virBitmap APIs, the point of the test is to check that they APIs behave correctly. We should report a proper error instead of aborting.
There are already tests which check proper extension of the bitmap. This is merely to extend it for testing that the bitmap is stringified correctly so I wouldn't consider this a part of the test.
I can lessen my requirement to 'return -1;' here, without bothering to write an error. But we should not abort on other than allocation errors in the tests - one pass through the test suite should execute all the tests instead of having developers dig through possible errors one-by-one. (Yes, this is currently the last tests, but sets a bad example) Jano
+ + if (!(res_set = virBitmapToString(map)) || + STRNEQ_NULLABLE(res_set, "804")) { + fprintf(stderr, "\n expected bitmap string '%s' actual string '%s'\n", + "804", NULLSTR(res_set)); + return -1; + } + + return 0; +} + + #define TESTBINARYOP(A, B, RES, FUNC) \ testBinaryOpData.a = A; \ testBinaryOpData.b = B; \
With the error message added: Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Modify the condition which would make virBitmapNewQuiet fail to possibly overallocate by 1 rather than failing. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5df7ea7838..a49019f884 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -55,8 +55,7 @@ struct _virBitmap { * * Allocate a bitmap capable of containing @size bits. * - * Returns a pointer to the allocated bitmap or NULL if memory cannot be - * allocated. Does not report libvirt errors. + * Returns a pointer to the allocated bitmap. */ virBitmapPtr virBitmapNewQuiet(size_t size) @@ -64,10 +63,13 @@ virBitmapNewQuiet(size_t size) virBitmapPtr bitmap; size_t sz; - if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size) - return NULL; - - sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT); + if (SIZE_MAX - VIR_BITMAP_BITS_PER_UNIT < size) { + /* VIR_DIV_UP would overflow, let's overallocate by 1 entry instead of + * the potential overflow */ + sz = (size / VIR_BITMAP_BITS_PER_UNIT) + 1; + } else { + sz = VIR_DIV_UP(size, VIR_BITMAP_BITS_PER_UNIT); + } bitmap = g_new0(virBitmap, 1); -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Modify the condition which would make virBitmapNewQuiet fail to possibly overallocate by 1 rather than failing.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We no longer report any errors so all callers can be replaced by virBitmapNew. Additionally virBitmapNEw can't return NULL now so error handling is not necessary. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 25 ++----------------------- src/util/virbitmap.h | 1 - src/util/virjson.c | 3 +-- tools/virt-host-validate-common.c | 4 +--- 5 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 4e66385bab..185f120f6b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1684,7 +1684,6 @@ virBitmapNew; virBitmapNewCopy; virBitmapNewData; virBitmapNewEmpty; -virBitmapNewQuiet; virBitmapNewString; virBitmapNextClearBit; virBitmapNextSetBit; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index a49019f884..5d224a8def 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -50,7 +50,7 @@ struct _virBitmap { /** - * virBitmapNewQuiet: + * virBitmapNew: * @size: number of bits * * Allocate a bitmap capable of containing @size bits. @@ -58,7 +58,7 @@ struct _virBitmap { * Returns a pointer to the allocated bitmap. */ virBitmapPtr -virBitmapNewQuiet(size_t size) +virBitmapNew(size_t size) { virBitmapPtr bitmap; size_t sz; @@ -84,27 +84,6 @@ virBitmapNewQuiet(size_t size) } -/** - * virBitmapNew: - * @size: number of bits - * - * Allocate a bitmap capable of containing @size bits. - * - * Returns a pointer to the allocated bitmap or NULL if either memory cannot be - * allocated or size is 0. Reports libvirt errors. - */ -virBitmapPtr -virBitmapNew(size_t size) -{ - virBitmapPtr ret; - - if (!(ret = virBitmapNewQuiet(size))) - virReportOOMError(); - - return ret; -} - - /** * virBitmapNewEmpty: * diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 615ec6e4af..57264fef0e 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -32,7 +32,6 @@ typedef virBitmap *virBitmapPtr; /* * Allocate a bitmap capable of containing @size bits. */ -virBitmapPtr virBitmapNewQuiet(size_t size) G_GNUC_WARN_UNUSED_RESULT; virBitmapPtr virBitmapNew(size_t size) G_GNUC_WARN_UNUSED_RESULT; virBitmapPtr virBitmapNewEmpty(void) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/util/virjson.c b/src/util/virjson.c index ba43d6b667..a4fc9e990e 100644 --- a/src/util/virjson.c +++ b/src/util/virjson.c @@ -1257,8 +1257,7 @@ virJSONValueGetArrayAsBitmap(const virJSONValue *val, maxelem = elems[i]; } - if (!(*bitmap = virBitmapNewQuiet(maxelem + 1))) - return -1; + *bitmap = virBitmapNew(maxelem + 1); /* second pass sets the correct bits in the map */ for (i = 0; i < val->data.array.nvalues; i++) diff --git a/tools/virt-host-validate-common.c b/tools/virt-host-validate-common.c index e0e22a133a..c3784bb91d 100644 --- a/tools/virt-host-validate-common.c +++ b/tools/virt-host-validate-common.c @@ -196,8 +196,7 @@ virBitmapPtr virHostValidateGetCPUFlags(void) if (!(fp = fopen("/proc/cpuinfo", "r"))) return NULL; - if (!(flags = virBitmapNewQuiet(VIR_HOST_VALIDATE_CPU_FLAG_LAST))) - goto cleanup; + flags = virBitmapNew(VIR_HOST_VALIDATE_CPU_FLAG_LAST); do { char line[1024]; @@ -245,7 +244,6 @@ virBitmapPtr virHostValidateGetCPUFlags(void) virStringListFreeCount(tokens, ntokens); } while (1); - cleanup: VIR_FORCE_FCLOSE(fp); return flags; -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
We no longer report any errors so all callers can be replaced by virBitmapNew. Additionally virBitmapNEw can't return NULL now so error
s/NEw/New/
handling is not necessary.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 25 ++----------------------- src/util/virbitmap.h | 1 - src/util/virjson.c | 3 +-- tools/virt-host-validate-common.c | 4 +--- 5 files changed, 4 insertions(+), 30 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
We now always return a valid pointer or crash so the return value doesn't need to be checked. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 57264fef0e..88af3bbadc 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -32,7 +32,7 @@ typedef virBitmap *virBitmapPtr; /* * Allocate a bitmap capable of containing @size bits. */ -virBitmapPtr virBitmapNew(size_t size) G_GNUC_WARN_UNUSED_RESULT; +virBitmapPtr virBitmapNew(size_t size); virBitmapPtr virBitmapNewEmpty(void) G_GNUC_WARN_UNUSED_RESULT; /* -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
We now always return a valid pointer or crash so the return value doesn't need to be checked.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
It can be replaced by virBitmapNew(0). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 2 +- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 16 +--------------- src/util/virbitmap.h | 1 - src/util/virhostcpu.c | 4 ++-- src/util/virnuma.c | 2 +- src/util/virtpm.c | 2 +- tests/virbitmaptest.c | 8 ++++---- 8 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c index cc6c77f105..b5ddf2c134 100644 --- a/src/conf/numa_conf.c +++ b/src/conf/numa_conf.c @@ -1374,7 +1374,7 @@ virDomainNumaDefValidate(const virDomainNuma *def) for (i = 0; i < def->nmem_nodes; i++) { const virDomainNumaNode *node = &def->mem_nodes[i]; - g_autoptr(virBitmap) levelsSeen = virBitmapNewEmpty(); + g_autoptr(virBitmap) levelsSeen = virBitmapNew(0); for (j = 0; j < node->ncaches; j++) { const virDomainNumaCache *cache = &node->caches[j]; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 185f120f6b..01ae3cad93 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1683,7 +1683,6 @@ virBitmapLastSetBit; virBitmapNew; virBitmapNewCopy; virBitmapNewData; -virBitmapNewEmpty; virBitmapNewString; virBitmapNextClearBit; virBitmapNextSetBit; diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 5d224a8def..1bff5d4ae6 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -84,20 +84,6 @@ virBitmapNew(size_t size) } -/** - * virBitmapNewEmpty: - * - * Allocate an empty bitmap. It can be used with self-expanding APIs. - * - * Returns a pointer to the allocated bitmap. - */ -virBitmapPtr -virBitmapNewEmpty(void) -{ - return g_new0(virBitmap, 1); -} - - /** * virBitmapFree: * @bitmap: previously allocated bitmap @@ -576,7 +562,7 @@ virBitmapParse(const char *str, virBitmapPtr virBitmapParseUnlimited(const char *str) { - virBitmapPtr bitmap = virBitmapNewEmpty(); + virBitmapPtr bitmap = virBitmapNew(0); bool neg = false; const char *cur = str; char *tmp; diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h index 88af3bbadc..cd5caa6f61 100644 --- a/src/util/virbitmap.h +++ b/src/util/virbitmap.h @@ -33,7 +33,6 @@ typedef virBitmap *virBitmapPtr; * Allocate a bitmap capable of containing @size bits. */ virBitmapPtr virBitmapNew(size_t size); -virBitmapPtr virBitmapNewEmpty(void) G_GNUC_WARN_UNUSED_RESULT; /* * Free previously allocated bitmap diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index f7e92cd924..c00a393def 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -336,7 +336,7 @@ virHostCPUParseNode(const char *node, goto cleanup; /* enumerate sockets in the node */ - sockets_map = virBitmapNewEmpty(); + sockets_map = virBitmapNew(0); while ((direrr = virDirRead(cpudir, &cpudirent, node)) > 0) { if (sscanf(cpudirent->d_name, "cpu%u", &cpu) != 1) @@ -372,7 +372,7 @@ virHostCPUParseNode(const char *node, goto cleanup; for (i = 0; i < sock_max; i++) - cores_maps[i] = virBitmapNewEmpty(); + cores_maps[i] = virBitmapNew(0); /* Iterate over all CPUs in the node, in ascending order */ for (cpu = 0; cpu < npresent_cpus; cpu++) { diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 39f0f30917..5a1218cdf2 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -311,7 +311,7 @@ virNumaNodesetToCPUset(virBitmapPtr nodeset, if (!nodeset) return 0; - allNodesCPUs = virBitmapNewEmpty(); + allNodesCPUs = virBitmapNew(0); nodesetSize = virBitmapSize(nodeset); for (i = 0; i < nodesetSize; i++) { diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 1a3b5a05aa..0e11674a3c 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -183,7 +183,7 @@ virTPMExecGetCaps(virCommandPtr cmd, if (virCommandRun(cmd, &exitstatus) < 0) return NULL; - bitmap = virBitmapNewEmpty(); + bitmap = virBitmapNew(0); /* older version does not support --print-capabilties -- that's fine */ if (exitstatus != 0) { diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index c14a6c7e26..255492c9e8 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -178,7 +178,7 @@ test4(const void *data G_GNUC_UNUSED) /* 0. empty set */ - bitmap = virBitmapNewEmpty(); + bitmap = virBitmapNew(0); if (virBitmapNextSetBit(bitmap, -1) != -1) return -1; @@ -571,7 +571,7 @@ test11(const void *opaque) static int test12(const void *opaque G_GNUC_UNUSED) { - g_autoptr(virBitmap) map = virBitmapNewEmpty(); + g_autoptr(virBitmap) map = virBitmapNew(0); TEST_MAP(0, ""); @@ -684,11 +684,11 @@ test15(const void *opaque) } -/* virBitmapNewEmpty + virBitmapToString */ +/* virBitmapNew(0) + virBitmapToString */ static int test16(const void *opaque G_GNUC_UNUSED) { - g_autoptr(virBitmap) map = virBitmapNewEmpty(); + g_autoptr(virBitmap) map = virBitmapNew(0); g_autofree char *res_empty = NULL; g_autofree char *res_set = NULL; -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
It can be replaced by virBitmapNew(0).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 2 +- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 16 +--------------- src/util/virbitmap.h | 1 - src/util/virhostcpu.c | 4 ++-- src/util/virnuma.c | 2 +- src/util/virtpm.c | 2 +- tests/virbitmaptest.c | 8 ++++---- 8 files changed, 10 insertions(+), 26 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Also, s/bitmp/bitmap/ in the commit summary. Jano On a Friday in 2020, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
It can be replaced by virBitmapNew(0).
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/numa_conf.c | 2 +- src/libvirt_private.syms | 1 - src/util/virbitmap.c | 16 +--------------- src/util/virbitmap.h | 1 - src/util/virhostcpu.c | 4 ++-- src/util/virnuma.c | 2 +- src/util/virtpm.c | 2 +- tests/virbitmaptest.c | 8 ++++---- 8 files changed, 10 insertions(+), 26 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_slirp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c index 3efc34bcf0..d9105b0ff6 100644 --- a/src/qemu/qemu_slirp.c +++ b/src/qemu/qemu_slirp.c @@ -81,10 +81,9 @@ qemuSlirpHasFeature(const qemuSlirp *slirp, qemuSlirpPtr qemuSlirpNew(void) { - g_autoptr(qemuSlirp) slirp = NULL; + g_autoptr(qemuSlirp) slirp = g_new0(qemuSlirp, 1); - if (VIR_ALLOC(slirp) < 0 || - !(slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST))) + if (!(slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST))) return NULL; slirp->pid = (pid_t)-1; -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_slirp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Remove return value check from all callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 7 ++----- src/conf/checkpoint_conf.c | 3 +-- src/conf/domain_addr.c | 6 ++---- src/conf/domain_conf.c | 12 ++++-------- src/conf/node_device_conf.c | 6 ++---- src/conf/storage_conf.c | 3 +-- src/conf/virnetworkobj.c | 3 +-- src/libxl/libxl_capabilities.c | 4 ---- src/libxl/libxl_driver.c | 6 ++---- src/lxc/lxc_controller.c | 3 --- src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_conf.c | 3 +-- src/qemu/qemu_domain_address.c | 5 +---- src/qemu/qemu_driver.c | 11 +++-------- src/qemu/qemu_hotplug.c | 3 +-- src/qemu/qemu_migration_cookie.c | 5 ++--- src/qemu/qemu_migration_params.c | 12 ++---------- src/qemu/qemu_monitor.c | 3 +-- src/qemu/qemu_namespace.c | 5 ++--- src/qemu/qemu_process.c | 5 +---- src/qemu/qemu_slirp.c | 4 +--- src/qemu/qemu_snapshot.c | 3 +-- src/test/test_driver.c | 12 +++--------- src/util/virbitmap.c | 12 ++---------- src/util/vircommand.c | 3 +-- src/util/virdnsmasq.c | 7 +------ src/util/virhostcpu.c | 10 +++------- src/util/virnetdev.c | 3 +-- src/util/virnuma.c | 6 ++---- src/util/virportallocator.c | 6 +----- src/util/virprocess.c | 6 ++---- src/util/virresctrl.c | 10 +--------- src/util/virstoragefile.c | 3 +-- src/vmx/vmx.c | 2 -- src/vz/vz_sdk.c | 3 +-- tests/qemumonitorjsontest.c | 3 --- tests/testutils.c | 11 ++--------- tests/virbitmaptest.c | 12 ++---------- tools/virsh-domain.c | 9 +++------ 39 files changed, 58 insertions(+), 175 deletions(-) diff --git a/src/conf/capabilities.c b/src/conf/capabilities.c index 610e6e8242..4a85c63628 100644 --- a/src/conf/capabilities.c +++ b/src/conf/capabilities.c @@ -1411,9 +1411,7 @@ virCapabilitiesHostNUMAGetCpus(virCapsHostNUMAPtr caps, unsigned int maxcpu = virCapabilitiesHostNUMAGetMaxcpu(caps); ssize_t node = -1; - if (!(ret = virBitmapNew(maxcpu + 1))) - return NULL; - + ret = virBitmapNew(maxcpu + 1); while ((node = virBitmapNextSetBit(nodemask, node)) >= 0) { if (virCapabilitiesHostNUMAGetCellCpus(caps, node, ret) < 0) { @@ -1591,8 +1589,7 @@ virCapabilitiesHostNUMAInitFake(virCapsHostNUMAPtr caps) cpus[cid].die_id = 0; cpus[cid].socket_id = s; cpus[cid].core_id = c; - if (!(cpus[cid].siblings = virBitmapNew(ncpus))) - goto error; + cpus[cid].siblings = virBitmapNew(ncpus); virBitmapCopy(cpus[cid].siblings, siblings); cid++; } diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 914af4305d..f97621dabf 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -323,8 +323,7 @@ virDomainCheckpointAlignDisks(virDomainCheckpointDefPtr def) if (!def->ndisks) checkpoint_default = VIR_DOMAIN_CHECKPOINT_TYPE_BITMAP; - if (!(map = virBitmapNew(def->parent.dom->ndisks))) - goto cleanup; + map = virBitmapNew(def->parent.dom->ndisks); /* Double check requested disks. */ for (i = 0; i < def->ndisks; i++) { diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 6e77a72f7c..9114f2d8d0 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -1595,8 +1595,7 @@ virDomainVirtioSerialAddrSetAddController(virDomainVirtioSerialAddrSetPtr addrs, if (VIR_ALLOC(cnt) < 0) goto cleanup; - if (!(cnt->ports = virBitmapNew(ports))) - goto cleanup; + cnt->ports = virBitmapNew(ports); cnt->idx = cont->idx; if ((insertAt = virDomainVirtioSerialAddrPlaceController(addrs, cnt)) < -1) @@ -2043,8 +2042,7 @@ virDomainUSBAddressHubNew(size_t nports) if (VIR_ALLOC(hub) < 0) goto cleanup; - if (!(hub->portmap = virBitmapNew(nports))) - goto cleanup; + hub->portmap = virBitmapNew(nports); if (VIR_ALLOC_N(hub->ports, nports) < 0) goto cleanup; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b80d7c7c6c..2fbfd949fd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2032,12 +2032,9 @@ virDomainDefGetVcpus(const virDomainDef *def) virBitmapPtr virDomainDefGetOnlineVcpumap(const virDomainDef *def) { - virBitmapPtr ret = NULL; + virBitmapPtr ret = virBitmapNew(def->maxvcpus); size_t i; - if (!(ret = virBitmapNew(def->maxvcpus))) - return NULL; - for (i = 0; i < def->maxvcpus; i++) { if (def->vcpus[i]->online) ignore_value(virBitmapSetBit(ret, i)); @@ -3311,8 +3308,7 @@ virDomainIOThreadIDDefArrayInit(virDomainDefPtr def, return 0; /* iothread's are numbered starting at 1, account for that */ - if (!(thrmap = virBitmapNew(iothreads + 1))) - return -1; + thrmap = virBitmapNew(iothreads + 1); virBitmapSetAll(thrmap); /* Clear 0 since we don't use it, then mark those which are @@ -4505,8 +4501,8 @@ virDomainDefRejectDuplicateControllers(virDomainDefPtr def) max_idx[VIR_DOMAIN_CONTROLLER_TYPE_USB] = -1; for (i = 0; i < VIR_DOMAIN_CONTROLLER_TYPE_LAST; i++) { - if (max_idx[i] >= 0 && !(bitmaps[i] = virBitmapNew(max_idx[i] + 1))) - goto cleanup; + if (max_idx[i] >= 0) + bitmaps[i] = virBitmapNew(max_idx[i] + 1); nbitmaps++; } diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index a9a03ad6c2..e478238675 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -1143,10 +1143,8 @@ virNodeDevCapNetParseXML(xmlXPathContextPtr ctxt, if ((n = virXPathNodeSet("./feature", ctxt, &nodes)) < 0) goto out; - if (n > 0) { - if (!(net->features = virBitmapNew(VIR_NET_DEV_FEAT_LAST))) - goto out; - } + if (n > 0) + net->features = virBitmapNew(VIR_NET_DEV_FEAT_LAST); for (i = 0; i < n; i++) { int val; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index d53d50479b..ae63cc725e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1444,8 +1444,7 @@ virStorageVolDefParseXML(virStoragePoolDefPtr pool, if (!def->target.compat) def->target.compat = g_strdup("1.1"); - if (!(def->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) - return NULL; + def->target.features = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST); for (i = 0; i < n; i++) { int f = virStorageFileFeatureTypeFromString((const char*)nodes[i]->name); diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 8b12ce0482..aefe23fde1 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -110,8 +110,7 @@ virNetworkObjNew(void) if (!(obj = virObjectLockableNew(virNetworkObjClass))) return NULL; - if (!(obj->classIdMap = virBitmapNew(INIT_CLASS_ID_BITMAP_SIZE))) - goto error; + obj->classIdMap = virBitmapNew(INIT_CLASS_ID_BITMAP_SIZE); /* The first three class IDs are already taken */ if (virBitmapSetBitExpand(obj->classIdMap, 0) < 0 || diff --git a/src/libxl/libxl_capabilities.c b/src/libxl/libxl_capabilities.c index e3472acb4b..622cba1bfc 100644 --- a/src/libxl/libxl_capabilities.c +++ b/src/libxl/libxl_capabilities.c @@ -296,10 +296,6 @@ libxlCapsInitNuma(libxl_ctx *ctx, virCapsPtr caps) cpus[node][nr_cpus_node[node]-1].core_id = cpu_topo[i].core; /* Allocate the siblings maps. We will be filling them later */ cpus[node][nr_cpus_node[node]-1].siblings = virBitmapNew(nr_cpus); - if (!cpus[node][nr_cpus_node[node]-1].siblings) { - virReportOOMError(); - goto cleanup; - } } /* Let's now populate the siblings bitmaps */ diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 80ee3ac5d0..446449fb33 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -2571,8 +2571,7 @@ libxlDomainGetVcpuPinInfo(virDomainPtr dom, int ncpumaps, /* Make sure coverity knows targetDef is valid at this point. */ sa_assert(targetDef); - if (!(hostcpus = virBitmapNew(libxl_get_max_cpus(cfg->ctx)))) - goto cleanup; + hostcpus = virBitmapNew(libxl_get_max_cpus(cfg->ctx)); virBitmapSetAll(hostcpus); ret = virDomainDefGetVcpuPinInfoHelper(targetDef, maplen, ncpumaps, cpumaps, @@ -4952,8 +4951,7 @@ libxlDomainGetNumaParameters(virDomainPtr dom, virReportOOMError(); goto cleanup; } - if (!(nodes = virBitmapNew(numnodes))) - goto cleanup; + nodes = virBitmapNew(numnodes); rc = libxl_domain_get_nodeaffinity(cfg->ctx, vm->def->id, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index c3cf485e2c..f70731bc64 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -756,9 +756,6 @@ static int virLXCControllerSetupCpuAffinity(virLXCControllerPtr ctrl) maxcpu = hostcpus; cpumap = virBitmapNew(maxcpu); - if (!cpumap) - return -1; - cpumapToSet = cpumap; if (ctrl->def->cpumask) { diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ea2e4c0948..e3a934363b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1800,8 +1800,7 @@ virQEMUCapsNew(void) return NULL; qemuCaps->invalidation = true; - if (!(qemuCaps->flags = virBitmapNew(QEMU_CAPS_LAST))) - goto error; + qemuCaps->flags = virBitmapNew(QEMU_CAPS_LAST); if (!(qemuCaps->domCapsCache = virQEMUDomainCapsCacheNew())) goto error; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index f4ed1fa061..2f5809e1ad 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -278,8 +278,7 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged, cfg->glusterDebugLevel = 4; cfg->stdioLogD = true; - if (!(cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST))) - return NULL; + cfg->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST); if (privileged && qemuDomainNamespaceAvailable(QEMU_DOMAIN_NS_MOUNT) && diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 61eb53ea2c..a2b19da5ea 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -3056,13 +3056,10 @@ qemuDomainUSBAddressAddHubs(virDomainDefPtr def) static virBitmapPtr qemuDomainGetMemorySlotMap(const virDomainDef *def) { - virBitmapPtr ret; + virBitmapPtr ret = virBitmapNew(def->mem.memory_slots); virDomainMemoryDefPtr mem; size_t i; - if (!(ret = virBitmapNew(def->mem.memory_slots))) - return NULL; - for (i = 0; i < def->nmems; i++) { mem = def->mems[i]; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 85b6a6a321..c23dca9970 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -19280,18 +19280,13 @@ qemuDomainGetGuestVcpusParams(virTypedParameterPtr *params, virTypedParameterPtr par = NULL; int npar = 0; int maxpar = 0; - virBitmapPtr vcpus = NULL; - virBitmapPtr online = NULL; - virBitmapPtr offlinable = NULL; + virBitmapPtr vcpus = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); + virBitmapPtr online = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); + virBitmapPtr offlinable = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID); g_autofree char *tmp = NULL; size_t i; int ret = -1; - if (!(vcpus = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID)) || - !(online = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID)) || - !(offlinable = virBitmapNew(QEMU_GUEST_VCPU_MAX_ID))) - goto cleanup; - for (i = 0; i < ninfo; i++) { if (virBitmapSetBit(vcpus, info[i].id) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 11b549b12b..86ccb9bac4 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -6150,8 +6150,7 @@ qemuDomainSelectHotplugVcpuEntities(virDomainDefPtr def, unsigned int curvcpus = virDomainDefGetVcpus(def); ssize_t i; - if (!(ret = virBitmapNew(maxvcpus))) - return NULL; + ret = virBitmapNew(maxvcpus); if (nvcpus > curvcpus) { *enable = true; diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index c128f541dd..4b73a98f7e 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1122,9 +1122,8 @@ qemuMigrationCookieCapsXMLParse(xmlXPathContextPtr ctxt) if (VIR_ALLOC(caps) < 0) return NULL; - if (!(caps->supported = virBitmapNew(QEMU_MIGRATION_CAP_LAST)) || - !(caps->automatic = virBitmapNew(QEMU_MIGRATION_CAP_LAST))) - goto cleanup; + caps->supported = virBitmapNew(QEMU_MIGRATION_CAP_LAST); + caps->automatic = virBitmapNew(QEMU_MIGRATION_CAP_LAST); if ((n = virXPathNodeSet("./capabilities[1]/cap", ctxt, &nodes)) < 0) goto cleanup; diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c index 5fde915963..df5560d39f 100644 --- a/src/qemu/qemu_migration_params.c +++ b/src/qemu/qemu_migration_params.c @@ -224,12 +224,9 @@ G_STATIC_ASSERT(G_N_ELEMENTS(qemuMigrationParamTypes) == QEMU_MIGRATION_PARAM_LA virBitmapPtr qemuMigrationParamsGetAlwaysOnCaps(qemuMigrationParty party) { - virBitmapPtr caps = NULL; + virBitmapPtr caps = virBitmapNew(QEMU_MIGRATION_CAP_LAST); size_t i; - if (!(caps = virBitmapNew(QEMU_MIGRATION_CAP_LAST))) - return NULL; - for (i = 0; i < G_N_ELEMENTS(qemuMigrationParamsAlwaysOn); i++) { if (!(qemuMigrationParamsAlwaysOn[i].party & party)) continue; @@ -248,8 +245,7 @@ qemuMigrationParamsNew(void) params = g_new0(qemuMigrationParams, 1); - if (!(params->caps = virBitmapNew(QEMU_MIGRATION_CAP_LAST))) - return NULL; + params->caps = virBitmapNew(QEMU_MIGRATION_CAP_LAST); return g_steal_pointer(¶ms); } @@ -1373,8 +1369,6 @@ qemuMigrationCapsCheck(virQEMUDriverPtr driver, return 0; priv->migrationCaps = virBitmapNew(QEMU_MIGRATION_CAP_LAST); - if (!priv->migrationCaps) - return -1; for (capStr = caps; *capStr; capStr++) { int cap = qemuMigrationCapabilityTypeFromString(*capStr); @@ -1389,8 +1383,6 @@ qemuMigrationCapsCheck(virQEMUDriverPtr driver, if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_MIGRATION_EVENT)) { migEvent = virBitmapNew(QEMU_MIGRATION_CAP_LAST); - if (!migEvent) - return -1; ignore_value(virBitmapSetBit(migEvent, QEMU_MIGRATION_CAP_EVENTS)); diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index eb4b04dc1a..c6be72dabc 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -1937,8 +1937,7 @@ qemuMonitorGetCpuHalted(qemuMonitorPtr mon, if (rc < 0) goto cleanup; - if (!(ret = virBitmapNew(maxvcpus))) - goto cleanup; + ret = virBitmapNew(maxvcpus); for (i = 0; i < ncpuentries; i++) { if (cpuentries[i].halted) diff --git a/src/qemu/qemu_namespace.c b/src/qemu/qemu_namespace.c index b0d1d0d083..ec52fe7ea5 100644 --- a/src/qemu/qemu_namespace.c +++ b/src/qemu/qemu_namespace.c @@ -799,9 +799,8 @@ qemuDomainEnableNamespace(virDomainObjPtr vm, { qemuDomainObjPrivatePtr priv = vm->privateData; - if (!priv->namespaces && - !(priv->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST))) - return -1; + if (!priv->namespaces) + priv->namespaces = virBitmapNew(QEMU_DOMAIN_NS_LAST); if (virBitmapSetBit(priv->namespaces, ns) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9122069cc9..8814e61314 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -5870,12 +5870,9 @@ qemuProcessValidateHotpluggableVcpus(virDomainDefPtr def) unsigned int maxvcpus = virDomainDefGetVcpusMax(def); size_t i = 0; size_t j; - virBitmapPtr ordermap = NULL; + virBitmapPtr ordermap = virBitmapNew(maxvcpus + 1); int ret = -1; - if (!(ordermap = virBitmapNew(maxvcpus + 1))) - goto cleanup; - /* validate: * - all hotpluggable entities to be hotplugged have the correct data * - vcpus belonging to a hotpluggable entity share configuration diff --git a/src/qemu/qemu_slirp.c b/src/qemu/qemu_slirp.c index d9105b0ff6..d2e4ed79be 100644 --- a/src/qemu/qemu_slirp.c +++ b/src/qemu/qemu_slirp.c @@ -83,9 +83,7 @@ qemuSlirpNew(void) { g_autoptr(qemuSlirp) slirp = g_new0(qemuSlirp, 1); - if (!(slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST))) - return NULL; - + slirp->features = virBitmapNew(QEMU_SLIRP_FEATURE_LAST); slirp->pid = (pid_t)-1; slirp->fd[0] = slirp->fd[1] = -1; diff --git a/src/qemu/qemu_snapshot.c b/src/qemu/qemu_snapshot.c index c2953c179b..47df102817 100644 --- a/src/qemu/qemu_snapshot.c +++ b/src/qemu/qemu_snapshot.c @@ -163,8 +163,7 @@ qemuSnapshotCreateInactiveExternal(virQEMUDriverPtr driver, if (!(qemuImgPath = qemuFindQemuImgBinary(driver))) goto cleanup; - if (!(created = virBitmapNew(snapdef->ndisks))) - goto cleanup; + created = virBitmapNew(snapdef->ndisks); /* If reuse is true, then qemuSnapshotPrepare already * ensured that the new files exist, and it was up to the user to diff --git a/src/test/test_driver.c b/src/test/test_driver.c index d582c207f4..af8fd03308 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1360,8 +1360,6 @@ testOpenDefault(virConnectPtr conn) } for (i = 0; i < 16; i++) { virBitmapPtr siblings = virBitmapNew(16); - if (!siblings) - goto error; ignore_value(virBitmapSetBit(siblings, i)); privconn->cells[i / 8].cpus[(i % 8)].id = i; privconn->cells[i / 8].cpus[(i % 8)].socket_id = i / 8; @@ -2788,8 +2786,7 @@ testDomainGetEmulatorPinInfo(virDomainPtr dom, } else if (def->cpumask) { cpumask = def->cpumask; } else { - if (!(bitmap = virBitmapNew(hostcpus))) - goto cleanup; + bitmap = virBitmapNew(hostcpus); virBitmapSetAll(bitmap); cpumask = bitmap; } @@ -2966,9 +2963,7 @@ static int testDomainGetVcpus(virDomainPtr domain, statbase = g_get_real_time(); hostcpus = VIR_NODEINFO_MAXCPUS(privconn->nodeInfo); - if (!(allcpumap = virBitmapNew(hostcpus))) - goto cleanup; - + allcpumap = virBitmapNew(hostcpus); virBitmapSetAll(allcpumap); /* Clamp to actual number of vcpus */ @@ -3081,8 +3076,7 @@ testDomainGetVcpuPinInfo(virDomainPtr dom, if (!(def = virDomainObjGetOneDef(privdom, flags))) goto cleanup; - if (!(hostcpus = virBitmapNew(VIR_NODEINFO_MAXCPUS(driver->nodeInfo)))) - goto cleanup; + hostcpus = virBitmapNew(VIR_NODEINFO_MAXCPUS(driver->nodeInfo)); virBitmapSetAll(hostcpus); ret = virDomainDefGetVcpuPinInfoHelper(def, maplen, ncpumaps, cpumaps, diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 1bff5d4ae6..74d1883d94 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -433,8 +433,7 @@ virBitmapParseSeparator(const char *str, size_t i; int start, last; - if (!(*bitmap = virBitmapNew(bitmapSize))) - return -1; + *bitmap = virBitmapNew(bitmapSize); if (!str) goto error; @@ -664,10 +663,7 @@ virBitmapParseUnlimited(const char *str) virBitmapPtr virBitmapNewCopy(virBitmapPtr src) { - virBitmapPtr dst; - - if ((dst = virBitmapNew(src->nbits)) == NULL) - return NULL; + virBitmapPtr dst = virBitmapNew(src->nbits); if (virBitmapCopy(dst, src) != 0) { virBitmapFree(dst); @@ -699,8 +695,6 @@ virBitmapNewData(const void *data, const unsigned char *bytes = data; bitmap = virBitmapNew(len * CHAR_BIT); - if (!bitmap) - return NULL; /* le64toh is not available, so we do the conversion by hand */ p = bitmap->map; @@ -1115,8 +1109,6 @@ virBitmapNewString(const char *string) } bitmap = virBitmapNew(len * 4); - if (!bitmap) - return NULL; for (i = 0; i < len; i++) { unsigned long nibble = g_ascii_xdigit_value(string[len - i - 1]); diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 76f7eb9a3d..31c8c79fb3 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -571,8 +571,7 @@ virCommandMassClose(virCommandPtr cmd, return -1; } - if (!(fds = virBitmapNew(openmax))) - return -1; + fds = virBitmapNew(openmax); # ifdef __linux__ if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0) diff --git a/src/util/virdnsmasq.c b/src/util/virdnsmasq.c index f60577b221..44aa7ad95d 100644 --- a/src/util/virdnsmasq.c +++ b/src/util/virdnsmasq.c @@ -768,14 +768,9 @@ dnsmasqCapsNewEmpty(const char *binaryPath) return NULL; if (!(caps = virObjectNew(dnsmasqCapsClass))) return NULL; - if (!(caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST))) - goto error; + caps->flags = virBitmapNew(DNSMASQ_CAPS_LAST); caps->binaryPath = g_strdup(binaryPath ? binaryPath : DNSMASQ); return caps; - - error: - virObjectUnref(caps); - return NULL; } dnsmasqCapsPtr diff --git a/src/util/virhostcpu.c b/src/util/virhostcpu.c index c00a393def..28f032b972 100644 --- a/src/util/virhostcpu.c +++ b/src/util/virhostcpu.c @@ -268,8 +268,7 @@ virHostCPUGetSiblingsList(unsigned int cpu) if (rv == -2) { /* If the file doesn't exist, the threadis its only sibling */ ret = virBitmapNew(cpu + 1); - if (ret) - ignore_value(virBitmapSetBit(ret, cpu)); + ignore_value(virBitmapSetBit(ret, cpu)); } return ret; @@ -332,8 +331,7 @@ virHostCPUParseNode(const char *node, goto cleanup; /* Keep track of the CPUs that belong to the current node */ - if (!(node_cpus_map = virBitmapNew(npresent_cpus))) - goto cleanup; + node_cpus_map = virBitmapNew(npresent_cpus); /* enumerate sockets in the node */ sockets_map = virBitmapNew(0); @@ -1119,9 +1117,7 @@ virHostCPUGetAvailableCPUsBitmap(void) if ((hostcpus = virHostCPUGetCount()) < 0) return NULL; - if (!(bitmap = virBitmapNew(hostcpus))) - return NULL; - + bitmap = virBitmapNew(hostcpus); virBitmapSetAll(bitmap); } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index c43823c747..76aeeba22a 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -3331,8 +3331,7 @@ virNetDevGetFeatures(const char *ifname, struct ifreq ifr; VIR_AUTOCLOSE fd = -1; - if (!(*out = virBitmapNew(VIR_NET_DEV_FEAT_LAST))) - return -1; + *out = virBitmapNew(VIR_NET_DEV_FEAT_LAST); if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1; diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 5a1218cdf2..6728f62a87 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -273,8 +273,7 @@ virNumaGetNodeCPUs(int node, return -2; } - if (!(cpumap = virBitmapNew(max_n_cpus))) - return -1; + cpumap = virBitmapNew(max_n_cpus); for (i = 0; i < max_n_cpus; i++) { if (MASK_CPU_ISSET(mask, i)) { @@ -1027,8 +1026,7 @@ virNumaGetHostMemoryNodeset(void) if (maxnode < 0) return NULL; - if (!(nodeset = virBitmapNew(maxnode + 1))) - return NULL; + nodeset = virBitmapNew(maxnode + 1); for (i = 0; i <= maxnode; i++) { if (!virNumaNodeIsAvailable(i)) diff --git a/src/util/virportallocator.c b/src/util/virportallocator.c index 2d34b617a6..f450740318 100644 --- a/src/util/virportallocator.c +++ b/src/util/virportallocator.c @@ -70,13 +70,9 @@ virPortAllocatorNew(void) if (!(pa = virObjectLockableNew(virPortAllocatorClass))) return NULL; - if (!(pa->bitmap = virBitmapNew(VIR_PORT_ALLOCATOR_NUM_PORTS))) - goto error; + pa->bitmap = virBitmapNew(VIR_PORT_ALLOCATOR_NUM_PORTS); return pa; - error: - virObjectUnref(pa); - return NULL; } static int diff --git a/src/util/virprocess.c b/src/util/virprocess.c index e9df563896..d379f7446f 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -520,8 +520,7 @@ virProcessGetAffinity(pid_t pid) goto cleanup; } - if (!(ret = virBitmapNew(ncpus))) - goto cleanup; + ret = virBitmapNew(ncpus); for (i = 0; i < ncpus; i++) { /* coverity[overrun-local] */ @@ -580,8 +579,7 @@ virProcessGetAffinity(pid_t pid) return NULL; } - if (!(ret = virBitmapNew(sizeof(mask) * 8))) - return NULL; + ret = virBitmapNew(sizeof(mask) * 8); for (i = 0; i < sizeof(mask) * 8; i++) if (CPU_ISSET(i, &mask)) diff --git a/src/util/virresctrl.c b/src/util/virresctrl.c index 2d945ff934..dabfa9990e 100644 --- a/src/util/virresctrl.c +++ b/src/util/virresctrl.c @@ -1157,13 +1157,9 @@ virResctrlAllocUpdateMask(virResctrlAllocPtr alloc, cache - a_type->nmasks + 1) < 0) return -1; - if (!a_type->masks[cache]) { + if (!a_type->masks[cache]) a_type->masks[cache] = virBitmapNew(virBitmapSize(mask)); - if (!a_type->masks[cache]) - return -1; - } - return virBitmapCopy(a_type->masks[cache], mask); } @@ -1874,8 +1870,6 @@ virResctrlAllocNewFromInfo(virResctrlInfoPtr info) virBitmapFree(mask); mask = virBitmapNew(i_type->bits); - if (!mask) - goto error; virBitmapSetAll(mask); for (k = 0; k <= i_type->max_cache_id; k++) { @@ -2102,8 +2096,6 @@ virResctrlAllocFindUnused(virResctrlAllocPtr alloc, } a_mask = virBitmapNew(i_type->bits); - if (!a_mask) - return -1; for (i = last_pos; i < last_pos + need_bits; i++) ignore_value(virBitmapSetBit(a_mask, i)); diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 229de14935..a669b4ca99 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -852,8 +852,7 @@ qcow2GetFeatures(virBitmapPtr *features, if (len < QCOW2v3_HDR_SIZE) return -1; - if (!(feat = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST))) - return -1; + feat = virBitmapNew(VIR_STORAGE_FILE_FEATURE_LAST); /* todo: check for incompatible or autoclear features? */ bits = virReadBufInt64BE(buf + QCOW2v3_HDR_FEATURES_COMPATIBLE); diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c index 4b1b04c6e1..fa4e685d97 100644 --- a/src/vmx/vmx.c +++ b/src/vmx/vmx.c @@ -1509,8 +1509,6 @@ virVMXParseConfig(virVMXContext *ctx, size_t naffs; def->cpumask = virBitmapNew(VIR_DOMAIN_CPUMASK_LEN); - if (!def->cpumask) - goto cleanup; if (!(afflist = virStringSplitCount(sched_cpu_affinity, ",", 0, &naffs))) goto cleanup; diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 3e7d55c50b..3e2f8ee01c 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -1481,8 +1481,7 @@ prlsdkConvertCpuInfo(PRL_HANDLE sdkdom, return -1; if (strlen(buf) == 0) { - if (!(def->cpumask = virBitmapNew(hostcpus))) - return -1; + def->cpumask = virBitmapNew(hostcpus); virBitmapSetAll(def->cpumask); } else { if (virBitmapParse(buf, &def->cpumask, hostcpus) < 0) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index a0dcbba07b..3973c762f0 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -2252,9 +2252,6 @@ testQemuMonitorJSONqemuMonitorJSONGetMigrationCapabilities(const void *opaque) } bitmap = virBitmapNew(QEMU_MIGRATION_CAP_LAST); - if (!bitmap) - goto cleanup; - ignore_value(virBitmapSetBit(bitmap, QEMU_MIGRATION_CAP_XBZRLE)); if (!(json = qemuMigrationCapsToJSON(bitmap, bitmap))) goto cleanup; diff --git a/tests/testutils.c b/tests/testutils.c index b747f65eea..2e586e98bf 100644 --- a/tests/testutils.c +++ b/tests/testutils.c @@ -832,8 +832,7 @@ int virTestMain(int argc, } } - if (!(failedTests = virBitmapNew(1))) - return EXIT_FAILURE; + failedTests = virBitmapNew(1); ret = (func)(); @@ -980,9 +979,7 @@ virTestCapsBuildNUMATopology(int seq) cell_cpus[core_id].id = id + core_id; cell_cpus[core_id].socket_id = cell_id + seq; cell_cpus[core_id].core_id = id + core_id; - if (!(cell_cpus[core_id].siblings = - virBitmapNew(MAX_CPUS_IN_CELL))) - goto error; + cell_cpus[core_id].siblings = virBitmapNew(MAX_CPUS_IN_CELL); ignore_value(virBitmapSetBit(cell_cpus[core_id].siblings, id)); } id++; @@ -997,10 +994,6 @@ virTestCapsBuildNUMATopology(int seq) } return g_steal_pointer(&caps); - - error: - VIR_FREE(cell_cpus); - return NULL; } static virDomainDefParserConfig virTestGenericDomainDefParserConfig = { diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c index 255492c9e8..5d2f1b0bad 100644 --- a/tests/virbitmaptest.c +++ b/tests/virbitmaptest.c @@ -36,8 +36,7 @@ test1(const void *data G_GNUC_UNUSED) size = 1024; bit = 100; - if (!(bitmap = virBitmapNew(size))) - return -1; + bitmap = virBitmapNew(size); if (virBitmapSetBit(bitmap, bit) < 0) return -1; @@ -140,8 +139,7 @@ test3(const void *data G_GNUC_UNUSED) int size = 5; size_t i; - if ((bitmap = virBitmapNew(size)) == NULL) - return -1; + bitmap = virBitmapNew(size); for (i = 0; i < size; i++) ignore_value(virBitmapSetBit(bitmap, i)); @@ -195,8 +193,6 @@ test4(const void *data G_GNUC_UNUSED) /* 1. zero set */ bitmap = virBitmapNew(size); - if (!bitmap) - return -1; if (virBitmapNextSetBit(bitmap, -1) != -1) return -1; @@ -337,8 +333,6 @@ test6(const void *v G_GNUC_UNUSED) int size = 64; bitmap = virBitmapNew(size); - if (!bitmap) - return -1; str = virBitmapFormat(bitmap); if (!str) @@ -416,8 +410,6 @@ test7(const void *v G_GNUC_UNUSED) for (i = 0; i < nmaxBit; i++) { g_autoptr(virBitmap) bitmap = virBitmapNew(maxBit[i]); - if (!bitmap) - return -1; if (virBitmapIsAllSet(bitmap)) return -1; diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index dfcba04682..b5e7d8f705 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -6840,8 +6840,7 @@ virshDomainGetVcpuBitmap(vshControl *ctl, if (curvcpus == 0) curvcpus = maxvcpus; - if (!(ret = virBitmapNew(maxvcpus))) - goto cleanup; + ret = virBitmapNew(maxvcpus); if ((nnodes = virXPathNodeSet("/domain/vcpus/vcpu", ctxt, &nodes)) <= 0) { /* if the specific vcpu state is missing provide a fallback */ @@ -7118,8 +7117,7 @@ virshParseCPUList(vshControl *ctl, int *cpumaplen, virBitmapPtr map = NULL; if (cpulist[0] == 'r') { - if (!(map = virBitmapNew(maxcpu))) - return NULL; + map = virBitmapNew(maxcpu); virBitmapSetAll(map); } else { int lastcpu; @@ -11989,8 +11987,7 @@ virshNodeIsSuperset(xmlNodePtr n1, xmlNodePtr n2) if (n1_child_size == 0 && n2_child_size == 0) return true; - if (!(bitmap = virBitmapNew(n1_child_size))) - return false; + bitmap = virBitmapNew(n1_child_size); child2 = n2->children; while (child2) { -- 2.26.2
On a Friday in 2020, Peter Krempa wrote:
Remove return value check from all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/capabilities.c | 7 ++----- src/conf/checkpoint_conf.c | 3 +-- src/conf/domain_addr.c | 6 ++---- src/conf/domain_conf.c | 12 ++++-------- src/conf/node_device_conf.c | 6 ++---- src/conf/storage_conf.c | 3 +-- src/conf/virnetworkobj.c | 3 +-- src/libxl/libxl_capabilities.c | 4 ---- src/libxl/libxl_driver.c | 6 ++---- src/lxc/lxc_controller.c | 3 --- src/qemu/qemu_capabilities.c | 3 +-- src/qemu/qemu_conf.c | 3 +-- src/qemu/qemu_domain_address.c | 5 +---- src/qemu/qemu_driver.c | 11 +++-------- src/qemu/qemu_hotplug.c | 3 +-- src/qemu/qemu_migration_cookie.c | 5 ++--- src/qemu/qemu_migration_params.c | 12 ++---------- src/qemu/qemu_monitor.c | 3 +-- src/qemu/qemu_namespace.c | 5 ++--- src/qemu/qemu_process.c | 5 +---- src/qemu/qemu_slirp.c | 4 +--- src/qemu/qemu_snapshot.c | 3 +-- src/test/test_driver.c | 12 +++--------- src/util/virbitmap.c | 12 ++---------- src/util/vircommand.c | 3 +-- src/util/virdnsmasq.c | 7 +------ src/util/virhostcpu.c | 10 +++------- src/util/virnetdev.c | 3 +-- src/util/virnuma.c | 6 ++---- src/util/virportallocator.c | 6 +----- src/util/virprocess.c | 6 ++---- src/util/virresctrl.c | 10 +--------- src/util/virstoragefile.c | 3 +-- src/vmx/vmx.c | 2 -- src/vz/vz_sdk.c | 3 +-- tests/qemumonitorjsontest.c | 3 --- tests/testutils.c | 11 ++--------- tests/virbitmaptest.c | 12 ++---------- tools/virsh-domain.c | 9 +++------ 39 files changed, 58 insertions(+), 175 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa