[vlc-devel] [PATCH 1/3] vlc_common.h: add helpers to get the upper rounded value of integer divisions

[Rémi Denis-Courmont]

On vendredi 17 mars 2017 10:46:07 EET Steve Lhomme wrote:
> On Fri, Mar 17, 2017 at 10:14 AM, Steve Lhomme <robux4 at gmail.com> wrote:
> > On Fri, Mar 17, 2017 at 9:46 AM, Rémi Denis-Courmont <remi at remlab.net> 
wrote:
> >> On March 17, 2017 4:36:08 PM GMT+08:00, Steve Lhomme 
<robux4 at videolabs.io> wrote:
> >>>---
> >>>
> >>> include/vlc_common.h | 5 +++++
> >>> 1 file changed, 5 insertions(+)
> >>>
> >>>diff --git a/include/vlc_common.h b/include/vlc_common.h
> >>>index 6a75753dbf..b72dbfe689 100644
> >>>--- a/include/vlc_common.h
> >>>+++ b/include/vlc_common.h
> >>>@@ -507,6 +507,11 @@ struct vlc_common_members
> >>>
> >>> /* clip v in [min, max] */
> >>> #define VLC_CLIP(v, min, max)    __MIN(__MAX((v), (min)), (max))
> >>>
> >>>+/* upper rounded value of the integer division */
> >>>+#define VLC_UPPER_DIV(v, div)     (((v) + (div) - 1) / (div))
> >>>+/* upper modulo value of the integer */
> >>>+#define VLC_UPPER_MODULO(v, mod)  (VLC_UPPER_DIV(v, mod) * mod)
> >>>+
> >>>
> >>> VLC_USED
> >>> static inline int64_t GCD ( int64_t a, int64_t b )
> >>> {
> >>>
> >>>--
> >>>2.11.1
> >>>
> >>>_______________________________________________
> >>>vlc-devel mailing list
> >>>To unsubscribe or modify your subscription options:
> >>>https://mailman.videolan.org/listinfo/vlc-devel
> >>>
> >> I don't like expansion-unsafe functional macros, TBH. They've bitten too
> >> many asses too many times, so to speak.> 
> > Reading on the subject, it seems the position of the parenthesis
> > matters (!). Would this be ok ?
> > #define VLC_UPPER_MODULO(v, mod)  ((mod) * VLC_UPPER_DIV(v, mod))
> > 
> > or probably better
> > 
> > #define VLC_UPPER_MODULO(v, mod)  ((v) + (v)%(mod))
> 
> Or not, it would be 1 minus the modulo, but only when it's not 0...

The expanded expression of an expansion-safe macro evaluates each of its 
parameters exactly once. This is not the case here, and that's why I dislike 
those "helpers".

This expression also won't work for negative numbers.

And it won't work in case of overflow. A more arithmetically correct 
computation would be:
	dividend + ((-dividend) % divisor)
This gives correct result in modulo arithmetic on overflow. But even that 
expression would not be expansion-safe. And it depends on European division 
algorithm, while C uses the obnoxious and stupid American algorithm.

Lastly, this macro, even if there was a way to fix it, will lead to suboptimal 
code if this common case: if the divisior has to be a power of two, but the 
compiler is unable to determine that at compile time. 

So I think we are better off with hand coding.

-- 
Rémi