diff --git a/pom.xml b/pom.xml index 7ea7b0112..18bf2c78d 100644 --- a/pom.xml +++ b/pom.xml @@ -165,6 +165,9 @@ Christopher Schuck + + Christian Semrau + Mauro Talevi diff --git a/src/java/org/apache/commons/math/MessagesResources_fr.java b/src/java/org/apache/commons/math/MessagesResources_fr.java index 927e50d43..804a7f672 100644 --- a/src/java/org/apache/commons/math/MessagesResources_fr.java +++ b/src/java/org/apache/commons/math/MessagesResources_fr.java @@ -44,6 +44,10 @@ public class MessagesResources_fr /** Non-translated/translated messages arrays. */ static final Object[][] contents = { + // org.apache.commons.math.util.MathUtils + { "overflow: gcd({0}, {1}) is 2^31", + "d\u00e9passement de capacit\u00e9 : le PGCD de {0} et {1} vaut 2^31" }, + // org.apache.commons.math.FunctionEvaluationException { "Evaluation failed for argument = {0}", "Erreur d''\u00e9valuation pour l''argument {0}" }, diff --git a/src/java/org/apache/commons/math/util/MathUtils.java b/src/java/org/apache/commons/math/util/MathUtils.java index 08df583de..806c888de 100644 --- a/src/java/org/apache/commons/math/util/MathUtils.java +++ b/src/java/org/apache/commons/math/util/MathUtils.java @@ -20,6 +20,9 @@ package org.apache.commons.math.util; import java.math.BigDecimal; import java.util.Arrays; +import org.apache.commons.math.MathException; +import org.apache.commons.math.MathRuntimeException; + /** * Some useful additions to the built-in functions in {@link Math}. * @version $Revision$ $Date$ @@ -510,14 +513,38 @@ public final class MathUtils { * operations. See Knuth 4.5.2 algorithm B. This algorithm is due to Josef * Stein (1961). *

+ * Special cases: + * * - * @param u a non-zero number - * @param v a non-zero number - * @return the greatest common divisor, never zero + * @param u any number + * @param v any number + * @return the greatest common divisor, never negative + * @throws ArithmeticException + * if the result cannot be represented as a nonnegative int + * value * @since 1.1 */ - public static int gcd(int u, int v) { + public static int gcd(final int p, final int q) { + int u = p; + int v = q; if ((u == 0) || (v == 0)) { + if ((u == Integer.MIN_VALUE) || (v == Integer.MIN_VALUE)) { + throw MathRuntimeException.createArithmeticException( + "overflow: gcd({0}, {1}) is 2^31", + new Object[] { p, q }); + } return (Math.abs(u) + Math.abs(v)); } // keep u and v negative, as negative integers range down to @@ -540,7 +567,9 @@ public final class MathUtils { k++; // cast out twos. } if (k == 31) { - throw new ArithmeticException("overflow: gcd is 2^31"); + throw MathRuntimeException.createArithmeticException( + "overflow: gcd({0}, {1}) is 2^31", + new Object[] { p, q }); } // B2. Initialize: u and v have been divided by 2^k and at least // one is odd. @@ -660,16 +689,37 @@ public final class MathUtils { } /** - * Returns the least common multiple between two integer values. + *

+ * Returns the least common multiple of the absolute value of two numbers, + * using the formula lcm(a,b) = (a / gcd(a,b)) * b. + *

+ * Special cases: + * * - * @param a the first integer value. - * @param b the second integer value. - * @return the least common multiple between a and b. - * @throws ArithmeticException if the lcm is too large to store as an int + * @param a any number + * @param b any number + * @return the least common multiple, never negative + * @throws ArithmeticException + * if the result cannot be represented as a nonnegative int + * value * @since 1.1 */ public static int lcm(int a, int b) { - return Math.abs(mulAndCheck(a / gcd(a, b), b)); + if (a==0 || b==0){ + return 0; + } + int lcm = Math.abs(mulAndCheck(a / gcd(a, b), b)); + if (lcm == Integer.MIN_VALUE){ + throw new ArithmeticException("overflow: lcm is 2^31"); + } + return lcm; } /** diff --git a/src/site/xdoc/changes.xml b/src/site/xdoc/changes.xml index ea2206528..ba3f36f04 100644 --- a/src/site/xdoc/changes.xml +++ b/src/site/xdoc/changes.xml @@ -39,6 +39,10 @@ The type attribute can be add,update,fix,remove. + + Fixed an error in computing gcd and lcm for some extreme values at integer + range boundaries. + Added a MathUtils method to check equality given some error bounds. diff --git a/src/test/org/apache/commons/math/util/MathUtilsTest.java b/src/test/org/apache/commons/math/util/MathUtilsTest.java index 26433d6b7..20b61df09 100644 --- a/src/test/org/apache/commons/math/util/MathUtilsTest.java +++ b/src/test/org/apache/commons/math/util/MathUtilsTest.java @@ -429,12 +429,28 @@ public final class MathUtilsTest extends TestCase { assertEquals(3 * (1<<15), MathUtils.gcd(3 * (1<<20), 9 * (1<<15))); assertEquals(Integer.MAX_VALUE, MathUtils.gcd(Integer.MAX_VALUE, 0)); - // abs(Integer.MIN_VALUE) == Integer.MIN_VALUE - assertEquals(Integer.MIN_VALUE, MathUtils.gcd(Integer.MIN_VALUE, 0)); + assertEquals(Integer.MAX_VALUE, MathUtils.gcd(-Integer.MAX_VALUE, 0)); + assertEquals(1<<30, MathUtils.gcd(1<<30, -Integer.MIN_VALUE)); try { - MathUtils.gcd(Integer.MIN_VALUE, Integer.MIN_VALUE); + // gcd(Integer.MIN_VALUE, 0) > Integer.MAX_VALUE + MathUtils.gcd(Integer.MIN_VALUE, 0); + fail("expecting ArithmeticException"); } catch (ArithmeticException expected) { - // + // expected + } + try { + // gcd(0, Integer.MIN_VALUE) > Integer.MAX_VALUE + MathUtils.gcd(0, Integer.MIN_VALUE); + fail("expecting ArithmeticException"); + } catch (ArithmeticException expected) { + // expected + } + try { + // gcd(Integer.MIN_VALUE, Integer.MIN_VALUE) > Integer.MAX_VALUE + MathUtils.gcd(Integer.MIN_VALUE, Integer.MIN_VALUE); + fail("expecting ArithmeticException"); + } catch (ArithmeticException expected) { + // expected } } @@ -558,8 +574,32 @@ public final class MathUtilsTest extends TestCase { assertEquals(150, MathUtils.lcm(a, b)); assertEquals(150, MathUtils.lcm(-a, b)); assertEquals(150, MathUtils.lcm(a, -b)); + assertEquals(150, MathUtils.lcm(-a, -b)); assertEquals(2310, MathUtils.lcm(a, c)); + // Assert that no intermediate value overflows: + // The naive implementation of lcm(a,b) would be (a*b)/gcd(a,b) + assertEquals((1<<20)*15, MathUtils.lcm((1<<20)*3, (1<<20)*5)); + + // Special case + assertEquals(0, MathUtils.lcm(0, 0)); + + try { + // lcm == abs(MIN_VALUE) cannot be represented as a nonnegative int + MathUtils.lcm(Integer.MIN_VALUE, 1); + fail("Expecting ArithmeticException"); + } catch (ArithmeticException ex) { + // expected + } + + try { + // lcm == abs(MIN_VALUE) cannot be represented as a nonnegative int + MathUtils.lcm(Integer.MIN_VALUE, 1<<20); + fail("Expecting ArithmeticException"); + } catch (ArithmeticException ex) { + // expected + } + try { MathUtils.lcm(Integer.MAX_VALUE, Integer.MAX_VALUE - 1); fail("Expecting ArithmeticException");