From f82ca609d33ebd7ddd88ad581d16f1483c435e48 Mon Sep 17 00:00:00 2001 From: Han-Wen Nienhuys Date: Sat, 23 Dec 2006 20:55:47 +0000 Subject: [PATCH] * numbers.c (scm_i_fraction_reduce): move logic into scm_i_make_ratio(), so fractions are only read. scm_i_fraction_reduce() modifies a fraction when reading it. A race condition might lead to fractions being corrupted by reading them concurrently. * numbers.h: remove SCM_FRACTION_SET_NUMERATOR, SCM_FRACTION_SET_DENOMINATOR, SCM_FRACTION_REDUCED_BIT, SCM_FRACTION_REDUCED_SET, SCM_FRACTION_REDUCED_CLEAR, SCM_FRACTION_REDUCED. --- libguile/ChangeLog | 16 ++++++++++++++++ libguile/numbers.c | 45 ++++++++++++++------------------------------- libguile/numbers.h | 8 -------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/libguile/ChangeLog b/libguile/ChangeLog index 68c2c8bf3..68b020754 100644 --- a/libguile/ChangeLog +++ b/libguile/ChangeLog @@ -1,3 +1,19 @@ +2006-12-23 Han-Wen Nienhuys + + * numbers.c (scm_i_fraction_reduce): move logic into + scm_i_make_ratio(), so fractions are only read. + scm_i_fraction_reduce() modifies a fraction when reading it. A + race condition might lead to fractions being corrupted by reading + them concurrently. + + Also, the REDUCED bit alters the SCM_CELL_TYPE(), making + comparisons between reduced and unreduced fractions go wrong. + + * numbers.h: remove SCM_FRACTION_SET_NUMERATOR, + SCM_FRACTION_SET_DENOMINATOR, SCM_FRACTION_REDUCED_BIT, + SCM_FRACTION_REDUCED_SET, SCM_FRACTION_REDUCED_CLEAR, + SCM_FRACTION_REDUCED. + 2006-12-16 Kevin Ryde * scmsigs.c (scm_raise): Use raise() rather than kill(), as this is diff --git a/libguile/numbers.c b/libguile/numbers.c index a5f30408f..a0ef29cdd 100644 --- a/libguile/numbers.c +++ b/libguile/numbers.c @@ -452,28 +452,21 @@ scm_i_make_ratio (SCM numerator, SCM denominator) /* No, it's a proper fraction. */ - return scm_double_cell (scm_tc16_fraction, - SCM_UNPACK (numerator), - SCM_UNPACK (denominator), 0); + { + SCM divisor = scm_gcd (numerator, denominator); + if (!(scm_is_eq (divisor, SCM_I_MAKINUM(1)))) + { + numerator = scm_divide (numerator, divisor); + denominator = scm_divide (denominator, divisor); + } + + return scm_double_cell (scm_tc16_fraction, + SCM_UNPACK (numerator), + SCM_UNPACK (denominator), 0); + } } #undef FUNC_NAME -static void scm_i_fraction_reduce (SCM z) -{ - if (!(SCM_FRACTION_REDUCED (z))) - { - SCM divisor; - divisor = scm_gcd (SCM_FRACTION_NUMERATOR (z), SCM_FRACTION_DENOMINATOR (z)); - if (!(scm_is_eq (divisor, SCM_I_MAKINUM(1)))) - { - /* is this safe? */ - SCM_FRACTION_SET_NUMERATOR (z, scm_divide (SCM_FRACTION_NUMERATOR (z), divisor)); - SCM_FRACTION_SET_DENOMINATOR (z, scm_divide (SCM_FRACTION_DENOMINATOR (z), divisor)); - } - SCM_FRACTION_REDUCED_SET (z); - } -} - double scm_i_fraction2double (SCM z) { @@ -2387,7 +2380,6 @@ SCM_DEFINE (scm_number_to_string, "number->string", 1, 1, 0, } else if (SCM_FRACTIONP (n)) { - scm_i_fraction_reduce (n); return scm_string_append (scm_list_3 (scm_number_to_string (SCM_FRACTION_NUMERATOR (n), radix), scm_from_locale_string ("/"), scm_number_to_string (SCM_FRACTION_DENOMINATOR (n), radix))); @@ -2441,7 +2433,6 @@ int scm_i_print_fraction (SCM sexp, SCM port, scm_print_state *pstate SCM_UNUSED) { SCM str; - scm_i_fraction_reduce (sexp); str = scm_number_to_string (sexp, SCM_UNDEFINED); scm_lfwrite (scm_i_string_chars (str), scm_i_string_length (str), port); scm_remember_upto_here_1 (str); @@ -3109,8 +3100,6 @@ scm_complex_equalp (SCM x, SCM y) SCM scm_i_fraction_equalp (SCM x, SCM y) { - scm_i_fraction_reduce (x); - scm_i_fraction_reduce (y); if (scm_is_false (scm_equal_p (SCM_FRACTION_NUMERATOR (x), SCM_FRACTION_NUMERATOR (y))) || scm_is_false (scm_equal_p (SCM_FRACTION_DENOMINATOR (x), @@ -5434,10 +5423,7 @@ scm_numerator (SCM z) else if (SCM_BIGP (z)) return z; else if (SCM_FRACTIONP (z)) - { - scm_i_fraction_reduce (z); - return SCM_FRACTION_NUMERATOR (z); - } + return SCM_FRACTION_NUMERATOR (z); else if (SCM_REALP (z)) return scm_exact_to_inexact (scm_numerator (scm_inexact_to_exact (z))); else @@ -5456,10 +5442,7 @@ scm_denominator (SCM z) else if (SCM_BIGP (z)) return SCM_I_MAKINUM (1); else if (SCM_FRACTIONP (z)) - { - scm_i_fraction_reduce (z); - return SCM_FRACTION_DENOMINATOR (z); - } + return SCM_FRACTION_DENOMINATOR (z); else if (SCM_REALP (z)) return scm_exact_to_inexact (scm_denominator (scm_inexact_to_exact (z))); else diff --git a/libguile/numbers.h b/libguile/numbers.h index 8448b7fd2..2c2fdcf07 100644 --- a/libguile/numbers.h +++ b/libguile/numbers.h @@ -157,14 +157,6 @@ #define SCM_FRACTIONP(x) (!SCM_IMP (x) && SCM_TYP16 (x) == scm_tc16_fraction) #define SCM_FRACTION_NUMERATOR(x) (SCM_CELL_OBJECT_1 (x)) #define SCM_FRACTION_DENOMINATOR(x) (SCM_CELL_OBJECT_2 (x)) -#define SCM_FRACTION_SET_NUMERATOR(x, v) (SCM_SET_CELL_OBJECT_1 ((x), (v))) -#define SCM_FRACTION_SET_DENOMINATOR(x, v) (SCM_SET_CELL_OBJECT_2 ((x), (v))) - - /* I think the left half word is free in the type, so I'll use bit 17 */ -#define SCM_FRACTION_REDUCED_BIT 0x10000 -#define SCM_FRACTION_REDUCED_SET(x) (SCM_SET_CELL_TYPE((x), (SCM_CELL_TYPE (x) | SCM_FRACTION_REDUCED_BIT))) -#define SCM_FRACTION_REDUCED_CLEAR(x) (SCM_SET_CELL_TYPE((x), (SCM_CELL_TYPE (x) & ~SCM_FRACTION_REDUCED_BIT))) -#define SCM_FRACTION_REDUCED(x) (0x10000 & SCM_CELL_TYPE (x))