salzman@randvax.UUCP (Isaac Salzman) (02/12/90)
[built with g++ 1.36.4 on a Sparcstation-1]
Itolong in Integer.cc is broken. For small Integers (those that take up only
1 short), there's a chance you'll get back garbage when you assign it to
a long. Here are the offending lines of code:
/* what if rep->s[1] isn't defined??!! */
unsigned long a = rep->s[SHORT_PER_LONG - 1];
if (a >= I_MINNUM)
return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG;
else
{
.
.
.
SHORT_PER_LONG ends up as 2, so if you're converting something that takes
only 1 short, there's a good chance that you'll get back MAXLONG or MINLONG,
depending on what happens to be in rep->s[1]. There are several ways to fix
this. However, Lou Moore (mooooore@rand.org) suggested a nice general
replacement for Itolong, so here are the context diff's:
*** Orig/Integer.cc Mon Feb 5 15:31:29 1990
--- Integer.cc Sun Feb 11 11:15:12 1990
***************
*** 343,374 ****
return rep;
}
-
// convert to a legal two's complement long if possible
// if too big, return most negative/positive value
long Itolong(const IntRep* rep)
{
! if (rep->len > SHORT_PER_LONG)
return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG;
else
{
! unsigned long a = rep->s[SHORT_PER_LONG - 1];
! if (a >= I_MINNUM)
! return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG;
! else
! {
! a = up(a) | rep->s[SHORT_PER_LONG - 2];
! if (SHORT_PER_LONG - 3 >= 0)
{
! for (int i = SHORT_PER_LONG - 3; i >= 0; --i)
! a = up(a) | rep->s[i];
}
! return (rep->sgn == I_POSITIVE)? a : -((long)a);
! }
}
}
-
// test whether op long() will work.
// careful about asymmetry between MINLONG & MAXLONG
--- 343,367 ----
return rep;
}
// convert to a legal two's complement long if possible
// if too big, return most negative/positive value
long Itolong(const IntRep* rep)
{
! int i = rep->len-1;
! if (i >= SHORT_PER_LONG)
return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG;
else
{
! unsigned long a = rep->s[i];
! while (i > 0)
{
! if ( (a = up(a) | rep->s[--i]) >= I_MINNUM)
! return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG;
}
! return (rep->sgn == I_POSITIVE)? a : -((long)a);
}
}
// test whether op long() will work.
// careful about asymmetry between MINLONG & MAXLONG
--
* Isaac J. Salzman ----
* The RAND Corporation - Information Sciences Dept. /o o/ /
* 1700 Main St., PO Box 2138, Santa Monica, CA 90406-2138 | v | |
* AT&T : +1 213-393-0411 x6421 or x7923 (ISL lab) _| |_/
* Internet : salzman@rand.org / | |
* UUCP : !uunet!rand.org!salzman | | | dl@G.OSWEGO.EDU (Doug Lea) (02/13/90)
Sorry for the error. Thanks for the fix. This is a classic example of undermanaged code evolution. Originally, the Integer class maintained the requirement that the minimum length of an Integer was SHORT_PER_LONG, but this was dropped in 1.36.3, in order to simplify the new 1.36.3 Integer<->IntRep interface. The Itolong code was not changed to reflect this. Sorry. -Doug
salzman@RAND.ORG (Isaac Salzman) (02/14/90)
>dl@g.oswego.edu (Doug Lea) writes: > >Sorry for the error. Thanks for the fix. I'm sorry for the error! The fix I sent out earlier was broken. Here's *my* original "fix", which really does fix the problem at hand. I'm not particularly fond of it (it's a hack) and I still think the code could be written better. But here's the one line patch that'll get things working (and i've tested it!).... -isaac (salzman@rand.org) *** Orig/Integer.cc Mon Feb 5 15:31:29 1990 --- Integer.cc Tue Feb 13 14:33:11 1990 *************** *** 353,359 **** return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG; else { ! unsigned long a = rep->s[SHORT_PER_LONG - 1]; if (a >= I_MINNUM) return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG; else --- 353,359 ---- return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG; else { ! unsigned long a = (rep->len == 1) ? 0 : rep->s[SHORT_PER_LONG - 1]; if (a >= I_MINNUM) return (rep->sgn == I_POSITIVE) ? MAXLONG : MINLONG; else