Code review comment for lp:~alan-griffiths/miral/fix-1717061

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> +unsigned clamp_dim(unsigned dim)
> +{
> + return std::min<unsigned long>(std::numeric_limits<long>::max(), dim);
> +}
>
> There's a mix of types here: unsigned int, unsigned long and signed long,
> which is causing me problems understanding. I know sizeof(unsigned int) <=
> sizeof(long), which implies to me that
>
> dim <= std::numeric_limits<unsigned>::max()
> <= std::numeric_limits<long>::max()

when, as Brandon encountered on armhf, long and int are the same size.

     std::numeric_limits<long>::max() <= std::numeric_limits<unsigned>::max()

> so the min() will always choose "dim". So I don't understand what it is
> clamping to.

In the above case it is clamping to std::numeric_limits<long>::max()

> +// For our convenience when doing calculations we clamp the dimensions of the
> aspect ratio
> +// so they will fit into a long without overflow.
> What "long"? AspectRatio is a pair of unsigned ints.

If std::numeric_limits<long>::max() <= dim <= std::numeric_limits<unsigned>::max() then dim doesn't fit in a long.

« Back to merge proposal