[PATCH 0/2] internal.h: Fix VIR_MULTIPLY_ADD_IS_OVERFLOW() macro when @add is zero
Green pipeline: https://gitlab.com/MichalPrivoznik/libvirt/-/pipelines/2990090049 Michal Prívozník (2): utiltest: Add MultiplyAddIsOverflow test case internal.h: Fix VIR_MULTIPLY_ADD_IS_OVERFLOW() macro when @add is zero src/internal.h | 2 +- tests/utiltest.c | 40 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) -- 2.56.0
From: Michal Privoznik <mprivozn@redhat.com> This is to make sure any future changes done to VIR_MULTIPLY_ADD_IS_OVERFLOW() macro don't break it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/utiltest.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/utiltest.c b/tests/utiltest.c index 0bef502a84..3d3e1a17ec 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -423,6 +423,39 @@ testGetSubIDs(const void *data G_GNUC_UNUSED) } +static int +testMultiplyAddIsOverflow(const void *data G_GNUC_UNUSED) +{ + unsigned long a = ULONG_MAX / 1024UL; + + if (VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX, a, 1024, 1023)) { + VIR_TEST_DEBUG("should not have failed (limit=%lu, value%lu, multiply=1024, add=1023).", + ULONG_MAX, a); + return -1; + } + + if (!VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX, a, 1024, 1024)) { + VIR_TEST_DEBUG("should have failed (limit=%lu, value=%lu, multiply=1024, add=1024).", + ULONG_MAX, a); + return -1; + } + + if (VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX - 1022, a, 1024, 1)) { + VIR_TEST_DEBUG("should have failed (limit=%lu, value=%lu, multiply=1024, add=1).", + ULONG_MAX - 1022, a); + return -1; + } + + if (!VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX - 1023, a, 1024, 1)) { + VIR_TEST_DEBUG("should have failed (limit=%lu, value=%lu, multiply=1024, add=1).", + ULONG_MAX - 1023, a); + return -1; + } + + return 0; +} + + static int mymain(void) { @@ -447,6 +480,7 @@ mymain(void) DO_TEST(KernelCmdlineNextParam); DO_TEST(KernelCmdlineMatchParam); DO_TEST(GetSubIDs); + DO_TEST(MultiplyAddIsOverflow); return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.56.0
On Mon, Oct 26, 2026 at 15:30:20 +0100, Michal Privoznik wrote:
From: Michal Privoznik <mprivozn@redhat.com>
This is to make sure any future changes done to VIR_MULTIPLY_ADD_IS_OVERFLOW() macro don't break it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/utiltest.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+)
diff --git a/tests/utiltest.c b/tests/utiltest.c index 0bef502a84..3d3e1a17ec 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -423,6 +423,39 @@ testGetSubIDs(const void *data G_GNUC_UNUSED) }
+static int +testMultiplyAddIsOverflow(const void *data G_GNUC_UNUSED) +{ + unsigned long a = ULONG_MAX / 1024UL; + + if (VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX, a, 1024, 1023)) { + VIR_TEST_DEBUG("should not have failed (limit=%lu, value%lu, multiply=1024, add=1023).", + ULONG_MAX, a); + return -1; + } + + if (!VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX, a, 1024, 1024)) { + VIR_TEST_DEBUG("should have failed (limit=%lu, value=%lu, multiply=1024, add=1024).", + ULONG_MAX, a); + return -1; + } + + if (VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX - 1022, a, 1024, 1)) { + VIR_TEST_DEBUG("should have failed (limit=%lu, value=%lu, multiply=1024, add=1).",
"should not have failed ..."
+ ULONG_MAX - 1022, a); + return -1; + } + + if (!VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX - 1023, a, 1024, 1)) { + VIR_TEST_DEBUG("should have failed (limit=%lu, value=%lu, multiply=1024, add=1).", + ULONG_MAX - 1023, a); + return -1; + } + + return 0; +} + + static int mymain(void) { @@ -447,6 +480,7 @@ mymain(void) DO_TEST(KernelCmdlineNextParam); DO_TEST(KernelCmdlineMatchParam); DO_TEST(GetSubIDs); + DO_TEST(MultiplyAddIsOverflow);
return result == 0 ? EXIT_SUCCESS : EXIT_FAILURE; }
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
From: Michal Privoznik <mprivozn@redhat.com> The aim of VIR_MULTIPLY_ADD_IS_OVERFLOW() macro is to check whether a multiplication and subsequent addition would overflow over given limit. For instance, if value * multiply + add would fit into say unsigned long. But because of the way the macro is written, a compiler warning may be thrown if 'add' is zero, i.e. we want to check just if value * multiply would overflow. Rewrite the offending part of the macro to make if work even in that case. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/internal.h | 2 +- tests/utiltest.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/internal.h b/src/internal.h index df311551a4..4ba42f330a 100644 --- a/src/internal.h +++ b/src/internal.h @@ -54,7 +54,7 @@ */ #define VIR_MULTIPLY_ADD_IS_OVERFLOW(limit, value, multiply, add) \ (G_UNLIKELY(((multiply) > 0 && (value) > (limit) / (multiply)) || \ - ((limit) - ((value) * (multiply)) < (add)))) + ((limit) - (add) < ((value) * (multiply))))) /* The library itself is allowed to use deprecated functions / * variables, so effectively undefine the deprecated attribute diff --git a/tests/utiltest.c b/tests/utiltest.c index 3d3e1a17ec..24d175f67a 100644 --- a/tests/utiltest.c +++ b/tests/utiltest.c @@ -452,6 +452,12 @@ testMultiplyAddIsOverflow(const void *data G_GNUC_UNUSED) return -1; } + if (VIR_MULTIPLY_ADD_IS_OVERFLOW(ULONG_MAX, a, 1024, 0)) { + VIR_TEST_DEBUG("should not have failed (limit=%lu, value=%lu, multiply=1024, add=0).", + ULONG_MAX, a); + return -1; + } + return 0; } -- 2.56.0
On Mon, Oct 26, 2026 at 15:30:21 +0100, Michal Privoznik wrote:
From: Michal Privoznik <mprivozn@redhat.com>
The aim of VIR_MULTIPLY_ADD_IS_OVERFLOW() macro is to check whether a multiplication and subsequent addition would overflow over given limit. For instance, if value * multiply + add would fit into say unsigned long. But because of the way the macro is written, a compiler warning may be thrown if 'add' is zero, i.e. we want to check just if value * multiply would overflow.
Rewrite the offending part of the macro to make if work even in that case.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/internal.h | 2 +- tests/utiltest.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
participants (2)
-
Jiri Denemark -
Michal Privoznik