libutils: util.h: corrections on some ROUND*() macros #7183
Conversation
Correct inline description comment for ROUNDUP(), ROUNDUP_OVERFLOW(), ROUNDDOWN() where the second argument is expected to be a power of 2, not a multiple of 2. Add an inline description comment to ROUNDUP_OVERFLOW() to state this requirement. Signed-off-by: Etienne Carriere <[email protected]>
…IV}() Assert condition on 2nd argument being a power of 2 is satisfied for ROUNDUP_OVERFLOW() and ROUNDUP_DIV() macros. Signed-off-by: Etienne Carriere <[email protected]>
Remove condition for 2nd argument to be a power of 2 for ROUNDUP() and ROUNDDOWN() macros. Signed-off-by: Etienne Carriere <[email protected]>
Fix indentation of the value defined for ROUNDDOWN() macro. Signed-off-by: Etienne Carriere <[email protected]>
Remove trailing space char in inline description comment of DIV_ROUND_UP() macro. Signed-off-by: Etienne Carriere <[email protected]>
Add selftest cases for ROUNDUP() and ROUNDDOWN() operations. Signed-off-by: Etienne Carriere <[email protected]>
| ~((__typeof__(v))(size) - 1)) | ||
| /* Round up the even multiple of size */ | ||
| #define ROUNDUP(v, size) \ | ||
| (IS_POWER_OF_TWO(size) ? \ |
There was a problem hiding this comment.
How about using two helper macros, one for the power of 2 case and one for the other?
While you're at it, it would be nice to evaluate the arguments only once, like in UDIV_ROUND_NEAREST() below.
Same below for ROUNDDOWN().
Would it be possible to fix ROUNDUP_OVERFLOW() to allow a size that isn't a power of 2? It would align better with the new versions of ROUNDUP() and ROUNDDOWN()
There was a problem hiding this comment.
How about using two helper macros, one for the power of 2 case and one for the other?
Okay, i'll go for ROUNDUP_POW2() and ROUNDUP() if that sounds ok to you (@jforissier ?)
That said, how to ensure one does not use a non power of 2 on ROUNDUP_POW2() for example. We cannot use assert() because the macro is used in uint8_t buf[ROUNDUP(foo, bar)] like array definitions?
One way: use non-pow2 ROUNDUP() macro in these cases (we expect the result is evaluated at build time).
Another way: have ROUNDUP_POW2() and ROUNDDOWN_POW2() to get the bit shift value instead of the power-of-2 size value.
While you're at it, it would be nice to evaluate the arguments only once, like in UDIV_ROUND_NEAREST() below.
Same below for ROUNDDOWN().
Ok, i'll see that.
Would it be possible to fix ROUNDUP_OVERFLOW() to allow a size that isn't a power of 2? It would align better with the new versions of ROUNDUP() and ROUNDDOWN()
I agree. I'll do that.
There was a problem hiding this comment.
How about using two helper macros, one for the power of 2 case and one for the other?
Okay, i'll go for
ROUNDUP_POW2()andROUNDUP()if that sounds ok to you (@jforissier ?)
I'm sorry, was unclear. Keep the features of the ROUNDUP() macro, but implement it based on two helper macros to make it more readable.
That said, how to ensure one does not use a non power of 2 on
ROUNDUP_POW2()for example. We cannot useassert()because the macro is used inuint8_t buf[ROUNDUP(foo, bar)]like array definitions? One way: use non-pow2ROUNDUP()macro in these cases (we expect the result is evaluated at build time). Another way: haveROUNDUP_POW2()andROUNDDOWN_POW2()to get the bit shift value instead of the power-of-2 size value.
I think that is overkill, it's better to have a macro that can be optimized by the compiler.
Correct inline description comments for ROUND*() macros that request an operand to be a power of 2, not a multiple of 2.
Assert this condition macros where assertion are applicable.
Propose a implementation for relax this constraint for
ROUNDUP()andROUNDDOWN()macros where we can't use assertions.