COMMON: Reduce overflow risk in Common::Rational += and -= operators

svn-id: r55839

1 files changed, 19 insertions, 7 deletions

diff --git a/common/rational.cpp b/common/rational.cpp
index f55c2dcfe3..af38dd9144 100644
--- a/common/rational.cpp
+++ b/common/rational.cpp
@@ -57,7 +57,7 @@ void Rational::cancel() {
 	// Cancel the fraction by dividing both the num and the denom
 	// by their greatest common divisor.
 
-	int gcd = Common::gcd(_num, _denom);
+	const int gcd = Common::gcd(_num, _denom);
 
 	_num   /= gcd;
 	_denom /= gcd;
@@ -78,8 +78,14 @@ Rational &Rational::operator=(int right) {
 }
 
 Rational &Rational::operator+=(const Rational &right) {
-	_num   = _num * right._denom + right._num * _denom;
-	_denom = _denom * right._denom;
+	// Cancel common factors to avoid unnecessary overflow.
+	// Note that the result is *not* always normalized.
+	const int gcd = Common::gcd(_denom, right._denom);
+
+	_num    = _num * (right._denom / gcd);
+	_denom  = _denom / gcd;
+	_num   += right._num * _denom;
+	_denom *= right._denom;
 
 	cancel();
 
@@ -87,8 +93,14 @@ Rational &Rational::operator+=(const Rational &right) {
 }
 
 Rational &Rational::operator-=(const Rational &right) {
-	_num   = _num * right._denom - right._num * _denom;
-	_denom = _denom * right._denom;
+	// Cancel common factors to avoid unnecessary overflow.
+	// Note that the result is *not* always normalized.
+	const int gcd = Common::gcd(_denom, right._denom);
+
+	_num    = _num * (right._denom / gcd);
+	_denom  = _denom / gcd;
+	_num   -= right._num * _denom;
+	_denom *= right._denom;
 
 	cancel();
 
@@ -98,8 +110,8 @@ Rational &Rational::operator-=(const Rational &right) {
 Rational &Rational::operator*=(const Rational &right) {
 	// Cross-cancel to avoid unnecessary overflow;
 	// the result then is automatically normalized
-	int gcd1 = Common::gcd(_num, right._denom);
-	int gcd2 = Common::gcd(right._num, _denom);
+	const int gcd1 = Common::gcd(_num, right._denom);
+	const int gcd2 = Common::gcd(right._num, _denom);
 
 	_num   = (_num    / gcd1) * (right._num    / gcd2);
 	_denom = (_denom  / gcd2) * (right._denom  / gcd1);