[Development] Fixing QRect::width() / height()

Bo Thorsen bo at vikingsoft.eu
Tue Mar 15 13:08:42 CET 2016


Den 15-03-2016 kl. 14:07 skrev Marc Mutz:
> Hi,
>
> No, this is not about the +1.
>
> QRect is internally represented as QPoint topLeft, bottomRight, which means it
> can hold rectangles for which width() and height(), returning ints, may
> overflow, e.g.
>
>     QRect{{INT_MIN, INT_MIN}, QPoint{0, 0}}
>     QRect{{INT_MIN, INT_MIN}, QPoint{INT_MAX, INT_MAX}}
>
> While these may seem like pathological cases that never occur in practice, the
> auto-test checks such rectangles, and if it didn't the next attacker would.
>
> It is therefore important to provide a fix width() and height().
>
> There are two options I can see:
>
>     qint64 width64() const;
>     unsigned int width??() const;
>
> The first returns the result as a 64-bit quantitiy, preserving the negative
> widths of unnormalised rectangles, and never overflowing, but possibly
> penalises 32-bit platforms without 64-bit register support.
>
> The second still overflows for left() == INT_MIN, right() == INT_MAX, because
> of the +1 (don't highjack this thread, you have been warned! :), would only
> work on normalized rects, but doesn't require a 64-bit quantitiy.
>
> Opinions on which one to choose? One? The other? Both?

There is another option that doesn't mean a change of signature: Bound 
the result. So if the real result is > INT_MAX, return INT_MAX. Same for 
INT_MIN.

Yes, it's not the correct result, but I completely agree with you that 
it's a theoretical problem. As long as it's documented in the width() I 
really don't see the problem with this solution.

Bo Thorsen,
Director, Viking Software.

-- 
Viking Software
Qt and C++ developers for hire
http://www.vikingsoft.eu



More information about the Development mailing list