On Tue, Jan 13, 2015 at 09:24:37PM -0800, Ben Pfaff wrote:
Can you say a few words about the assumptions? At first glance, some of
the functions seem to suffer from catastrophic loss of precision upon
integer overflow, e.g. doesn't that happen in decimal_multiply() if the
intermediate result is greater than ORD_MAX? (Is some kind of care
taken to make sure that this cannot happen?)Oh you are right. Actually decimal_multiply is unused, so I have removed it. But the same issue was in decimal_int_multiply. I have fixed this and added a test for it. Patch attached. J' -- PGP Public key ID: 1024D/2DE827B3 fingerprint = 8797 A26D 0854 2EAB 0285 A290 8A67 719C 2DE8 27B3 See http://sks-keyservers.net or any PGP keyserver for public key.
From b98f7a5cbb3f49f8104a641b17a9be87b6b851fc Mon Sep 17 00:00:00 2001 From: John Darrington <[email protected]> Date: Wed, 14 Jan 2015 10:08:10 +0100 Subject: [PATCH] Remove decimal_multiply. Fix overflow handling in decimal_int_multiply and add tests. --- src/math/decimal.c | 18 +++++++------- src/math/decimal.h | 1 - tests/math/decimal-test.c | 57 +++++++++++++++++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 20 deletions(-) diff --git a/src/math/decimal.c b/src/math/decimal.c index ba33e76..0655834 100644 --- a/src/math/decimal.c +++ b/src/math/decimal.c @@ -299,23 +299,23 @@ decimal_cmp (const struct decimal *d1, const struct decimal *d2) } -/* Multiply DEST by SRC */ -void -decimal_multiply (struct decimal *dest, const struct decimal *src) -{ - dest->ordinate *= src->ordinate; - dest->mantissa += src->mantissa; -} - - /* Multiply DEST by M */ void decimal_int_multiply (struct decimal *dest, ord_t m) { + if (m != 0) + while (llabs (dest->ordinate) > llabs (ORD_MAX / m)) + { + dest->ordinate /= 10; + dest->mantissa++; + } + dest->ordinate *= m; + reduce (dest); } + /* Divide DEST by M */ void decimal_int_divide (struct decimal *dest, ord_t m) diff --git a/src/math/decimal.h b/src/math/decimal.h index 2a3fd60..b5740f6 100644 --- a/src/math/decimal.h +++ b/src/math/decimal.h @@ -93,7 +93,6 @@ void normalise (struct decimal *d1, struct decimal *d2); void decimal_init (struct decimal *dec, ord_t ord, mant_t mant); void decimal_init_from_string (struct decimal *dec, const char *s); int decimal_cmp (const struct decimal *d1, const struct decimal *d2); -void decimal_multiply (struct decimal *dest, const struct decimal *src); void decimal_int_multiply (struct decimal *dest, ord_t m); void decimal_int_divide (struct decimal *dest, ord_t m); void decimal_divide (struct decimal *dest, const struct decimal *src); diff --git a/tests/math/decimal-test.c b/tests/math/decimal-test.c index aa4d244..3e1e64e 100644 --- a/tests/math/decimal-test.c +++ b/tests/math/decimal-test.c @@ -200,21 +200,26 @@ test_addition (const struct decimal *one_, const struct decimal *two) assert (strcmp (sdsum1, sdsum2) == 0); } + void -test_multiplication (const struct decimal *one_, const struct decimal *two) +test_multiplication (const struct decimal *d, int m) { - struct decimal one = *one_; - double d1 = decimal_to_double (&one); - double d2 = decimal_to_double (two); - - decimal_multiply (&one, two); - - double dprod = decimal_to_double (&one); - - assert (dprod == d1 * d2); + char b1[256]; + char b2[256]; + struct decimal dest = *d; + double x = decimal_to_double (&dest); + + decimal_int_multiply (&dest, m); + + double y = decimal_to_double (&dest); + + snprintf (b1, 256, "%g", m * x); + snprintf (b2, 256, "%g", y); + assert (0 == strcmp (b1, b2)); } + int main (int argc, char **argv) { @@ -306,5 +311,37 @@ main (int argc, char **argv) } + { + struct decimal m = {10, 0}; + + test_multiplication (&m, 11); + } + + + { + struct decimal m = {ORD_MAX - 2, 0}; + + test_multiplication (&m, 11); + } + + + { + struct decimal m = {34, 0}; + + test_multiplication (&m, 0); + } + + { + struct decimal m = {34, -20}; + + test_multiplication (&m, 33); + } + + { + struct decimal m = {304, 2}; + + test_multiplication (&m, -33); + } + return 0; } -- 1.7.10.4
signature.asc
Description: Digital signature
_______________________________________________ pspp-dev mailing list [email protected] https://lists.gnu.org/mailman/listinfo/pspp-dev
