git sha: (unavailable - RFC patch via email)
Author: Gaetan Rivet <[email protected]>
Subject: [RFC PATCH 04/12] util: Add safe u64 arithmetic operations
This commit adds helper functions for safe 64-bit unsigned integer
arithmetic operations with overflow detection for addition, subtraction,
and multiplication.
> diff --git a/lib/util.h b/lib/util.h
> index ef993626a1..18dfd6975a 100644
> --- a/lib/util.h
> +++ b/lib/util.h
[ ... ]
> +static inline uint64_t
> +ovs_u64_safemul(uint64_t a, uint64_t b)
> +{
> + static const uint64_t sqrt_u64_max = UINT64_C(1) << (64 / 2);
> +
> + if ((a >= sqrt_u64_max || b >= sqrt_u64_max) &&
> + a > 0 && UINT64_MAX / a < b) {
> + return UINT64_MAX;
> + } else {
> + return a * b;
> + }
> +}
Is the multiplication overflow detection logic correct when both operands
are at or above sqrt_u64_max? With sqrt_u64_max being 2^32, two values
of exactly 2^32 would multiply to 2^64, which overflows UINT64_MAX.
Could the sqrt_u64_max constant calculation be clearer about its intent?
The current form UINT64_C(1) << (64 / 2) might be confusing compared to
a more explicit approach.
> diff --git a/tests/test-util.c b/tests/test-util.c
> index 5d88d38f26..2650c9507e 100644
> --- a/tests/test-util.c
> +++ b/tests/test-util.c
[ ... ]
> + mul_cases[] = {
> + { 0, 0, 0, },
> + { 0, 2, 0, },
> + { 1, 2, 2, },
> + { 0, UINT64_MAX, 0, },
> + { UINT64_MAX, 0, 0, },
> + { -1, 0, 0, },
> + { 0, -1, 0, },
> + { -1, -1, UINT64_MAX, },
> + { 4294967296, 4294967296, UINT64_MAX, },
> + { UINT64_MAX / 2, UINT64_MAX / 2, UINT64_MAX, },
> + { 2, UINT64_MAX / 2, UINT64_MAX - 1, },
> + };
The test case with 4294967296 * 4294967296 expects UINT64_MAX, but does
ovs_u64_safemul handle this edge case correctly? This is exactly 2^32 *
2^32 = 2^64, which should overflow.
Does the test adequately cover the boundary condition where a * b equals
exactly UINT64_MAX + 1?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
