long->sal_Int32 in BigInt

And fix an issue discovered in the PPT parsing, where one of the
test files has dodgy values that trigger the assert in
BigInt::operator long().

Change-Id: Ic324ac38ecef4153cc434376317643ababe0a537
Reviewed-on: https://gerrit.libreoffice.org/47314Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

filter/source/msfilter/svdfppt.cxx

@@ -288,8 +288,9 @@ SvStream& ReadPptDocumentAtom(SvStream& rIn, PptDocumentAtom& rAtom)
        .ReadSChar( nShowComments );
     rAtom.aSlidesPageSize.Width() = nSlideX;
     rAtom.aSlidesPageSize.Height() = nSlideY;
-    rAtom.aNotesPageSize.Width() = nNoticeX;
-    rAtom.aNotesPageSize.Height() = nNoticeY;
+    // clamp dodgy data to avoid overflow in later calculations
+    rAtom.aNotesPageSize.Width() = std::min<sal_Int32>(nNoticeX, 65536);
+    rAtom.aNotesPageSize.Height() = std::min<sal_Int32>(nNoticeY, 65536);
     rAtom.eSlidesPageFormat = (PptPageFormat)nSlidePageFormat;
     rAtom.bEmbeddedTrueType = nEmbeddedTrueType;
     rAtom.bTitlePlaceholdersOmitted = nTitlePlaceHoldersOmitted;

include/tools/bigint.hxx

@@ -33,8 +33,8 @@ class Fraction;
 class SAL_WARN_UNUSED TOOLS_DLLPUBLIC BigInt
 {
 private:
-    long            nVal;
-    unsigned short  nNum[MAX_DIGITS];
+    sal_Int32       nVal;
+    sal_uInt16      nNum[MAX_DIGITS];
     sal_uInt8       nLen        : 5;    // current length
     bool            bIsNeg      : 1,    // Is Sign negative?
                     bIsBig      : 1,    // sal_True == BigInt
@@ -62,16 +62,7 @@ public:
     {
     }
 
-    BigInt(short nValue)
-        : nVal(nValue)
-        , nLen(0)
-        , bIsNeg(false)
-        , bIsBig(false)
-        , bIsSet(true)
-    {
-    }
-
-    BigInt(long nValue)
+    BigInt(sal_Int32 nValue)
         : nVal(nValue)
         , nLen(0)
         , bIsNeg(false)
@@ -80,6 +71,8 @@ public:
     {
     }
 
+    // despite sal_Int32 being a typedef for int, MSVC won't automatically use the BigInt(sal_Int32) constructor
+#ifdef _WIN32
     BigInt(int nValue)
         : nVal(nValue)
         , nLen(0)
@@ -88,31 +81,22 @@ public:
         , bIsSet(true)
     {
     }
+#endif
 
     BigInt( double nVal );
-
-    BigInt(sal_uInt16 nValue)
-        : nVal(nValue)
-        , nLen(0)
-        , bIsNeg(false)
-        , bIsBig(false)
-        , bIsSet(true)
-    {
-    }
-
     BigInt( sal_uInt32 nVal );
-#if SAL_TYPES_SIZEOFLONG < SAL_TYPES_SIZEOFLONGLONG
-    BigInt( long long nVal );
-#endif
+    BigInt( sal_Int64 nVal );
     BigInt( const BigInt& rBigInt );
     BigInt( const OUString& rString );
 
-    operator        short() const;
-    operator        long()  const;
-    operator        int()   const;
-    operator        double() const;
+    operator        sal_Int16() const;
     operator        sal_uInt16() const;
-    operator        sal_uIntPtr() const;
+    operator        sal_Int32() const;
+    operator        sal_uInt32() const;
+    operator        double() const;
+#if SAL_TYPES_SIZEOFLONG == 8
+    operator        long() const;
+#endif
 
     bool            IsSet() const { return bIsSet; }
     bool            IsNeg() const;
@@ -128,10 +112,7 @@ public:
     BigInt&         operator /=( const BigInt& rVal );
     BigInt&         operator %=( const BigInt& rVal );
 
-    BigInt&         operator  =( const short      nValue );
-    BigInt&         operator  =( const long       nValue );
-    BigInt&         operator  =( const int        nValue );
-    BigInt&         operator  =( const sal_uInt16 nValue );
+    BigInt&         operator  =( sal_Int32 nValue );
 
     friend inline   BigInt operator +( const BigInt& rVal1, const BigInt& rVal2 );
     friend inline   BigInt operator -( const BigInt& rVal1, const BigInt& rVal2 );
@@ -149,66 +130,50 @@ public:
     friend class Fraction;
 };
 
-inline BigInt::operator short() const
+inline BigInt::operator sal_Int16() const
 {
-    if ( !bIsBig && nVal >= SHRT_MIN && nVal <= SHRT_MAX )
-        return (short)nVal;
-    else
-        return 0;
-}
-
-inline BigInt::operator long() const
-{
-    if ( !bIsBig )
-        return nVal;
-    else
-        return 0;
-}
-
-inline BigInt::operator int() const
-{
-    if ( !bIsBig && (nVal == (long)(int)nVal) )
-        return (int)nVal;
-    else
-        return 0;
+    if ( !bIsBig && nVal >= SAL_MIN_INT16 && nVal <= SAL_MAX_INT16 )
+        return (sal_Int16)nVal;
+    assert(false && "out of range");
+    return 0;
 }
 
 inline BigInt::operator sal_uInt16() const
 {
-    if ( !bIsBig && nVal >= 0 && nVal <= (long)USHRT_MAX )
+    if ( !bIsBig && nVal >= 0 && nVal <= SAL_MAX_UINT16 )
         return (sal_uInt16)nVal;
-    else
-        return 0;
+    assert(false && "out of range");
+    return 0;
 }
 
-inline BigInt& BigInt::operator =( const short nValue )
+inline BigInt::operator sal_Int32() const
 {
-    bIsSet = true;
-    bIsBig = false;
-    nVal   = nValue;
-
-    return *this;
+    if ( !bIsBig && nVal >= SAL_MIN_INT32 && nVal <= SAL_MAX_INT32 )
+        return (sal_Int32)nVal;
+    assert(false && "out of range");
+    return 0;
 }
 
-inline BigInt& BigInt::operator =( const long nValue )
+inline BigInt::operator sal_uInt32() const
 {
-    bIsSet = true;
-    bIsBig = false;
-    nVal   = nValue;
-
-    return *this;
+    if ( !bIsBig && nVal >= 0 )
+        return (sal_uInt32)nVal;
+    assert(false && "out of range");
+    return 0;
 }
 
-inline BigInt& BigInt::operator =( const int nValue )
+#if SAL_TYPES_SIZEOFLONG == 8
+inline BigInt::operator long() const
 {
-    bIsSet = true;
-    bIsBig = false;
-    nVal   = nValue;
-
-    return *this;
+    // Clamp to int32 since long is int32 on Windows.
+    if ( !bIsBig && nVal >= SAL_MIN_INT32 && nVal <= SAL_MAX_INT32 )
+        return (long)nVal;
+    assert(false && "out of range");
+    return 0;
 }
+#endif
 
-inline BigInt& BigInt::operator =( const sal_uInt16 nValue )
+inline BigInt& BigInt::operator =( sal_Int32 nValue )
 {
     bIsSet = true;
     bIsBig = false;

tools/qa/cppunit/test_bigint.cxx

@@ -47,7 +47,7 @@ void BigIntTest::testConstructionFromLongLong()
         CPPUNIT_ASSERT(!bi.IsZero());
         CPPUNIT_ASSERT(!bi.IsNeg());
         CPPUNIT_ASSERT(bi.IsLong());
-        CPPUNIT_ASSERT_EQUAL(42L, static_cast<long>(bi));
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(42), static_cast<sal_Int32>(bi));
     }
 
     // small negative number
@@ -57,7 +57,7 @@ void BigIntTest::testConstructionFromLongLong()
         CPPUNIT_ASSERT(!bi.IsZero());
         CPPUNIT_ASSERT(bi.IsNeg());
         CPPUNIT_ASSERT(bi.IsLong());
-        CPPUNIT_ASSERT_EQUAL(-42L, static_cast<long>(bi));
+        CPPUNIT_ASSERT_EQUAL(static_cast<sal_Int32>(-42), static_cast<sal_Int32>(bi));
     }
 
 #if SAL_TYPES_SIZEOFLONG < SAL_TYPES_SIZEOFLONGLONG

tools/source/generic/bigint.cxx

@@ -27,10 +27,11 @@
 
 #include <string.h>
 
-static const long MY_MAXLONG  = 0x3fffffff;
-static const long MY_MINLONG  = -MY_MAXLONG;
-static const long MY_MAXSHORT = 0x00007fff;
-static const long MY_MINSHORT = -MY_MAXSHORT;
+/**
+ * The range in which we can perform add/sub without fear of overflow
+ */
+static const sal_Int32 MY_MAXLONG  = 0x3fffffff;
+static const sal_Int32 MY_MINLONG  = -MY_MAXLONG;
 
 /*
  * The algorithms for Addition, Subtraction, Multiplication and Division
@@ -50,7 +51,7 @@ void BigInt::MakeBigInt( const BigInt& rVal )
     }
     else
     {
-        long nTmp = rVal.nVal;
+        sal_Int32 nTmp = rVal.nVal;
 
         nVal   = rVal.nVal;
         bIsBig = true;
@@ -85,7 +86,7 @@ void BigInt::Normalize()
             else if ( nNum[1] & 0x8000 )
                 return;
             else
-                nVal = ((long)nNum[1] << 16) + nNum[0];
+                nVal = ((sal_Int32)nNum[1] << 16) + nNum[0];
 
             bIsBig = false;
 
@@ -175,10 +176,10 @@ void BigInt::AddLong( BigInt& rB, BigInt& rErg )
         }
 
         // Add numerals, starting from the back
-        long k;
-        long nZ = 0;
+        sal_Int32 k;
+        sal_Int32 nZ = 0;
         for (i = 0, k = 0; i < len; i++) {
-            nZ = (long)nNum[i] + (long)rB.nNum[i] + k;
+            nZ = (sal_Int32)nNum[i] + (sal_Int32)rB.nNum[i] + k;
             if (nZ & 0xff0000L)
                 k = 1;
             else
@@ -217,7 +218,7 @@ void BigInt::SubLong( BigInt& rB, BigInt& rErg )
     {
         int  i;
         char len;
-        long nZ, k;
+        sal_Int32 nZ, k;
 
         // if length of the two values differ, fill remaining positions
         // of the smaller value with zeros.
@@ -238,7 +239,7 @@ void BigInt::SubLong( BigInt& rB, BigInt& rErg )
         {
             for (i = 0, k = 0; i < len; i++)
             {
-                nZ = (long)nNum[i] - (long)rB.nNum[i] + k;
+                nZ = (sal_Int32)nNum[i] - (sal_Int32)rB.nNum[i] + k;
                 if (nZ < 0)
                     k = -1;
                 else
@@ -251,7 +252,7 @@ void BigInt::SubLong( BigInt& rB, BigInt& rErg )
         {
             for (i = 0, k = 0; i < len; i++)
             {
-                nZ = (long)rB.nNum[i] - (long)nNum[i] + k;
+                nZ = (sal_Int32)rB.nNum[i] - (sal_Int32)nNum[i] + k;
                 if (nZ < 0)
                     k = -1;
                 else
@@ -283,8 +284,8 @@ void BigInt::SubLong( BigInt& rB, BigInt& rErg )
 
 void BigInt::MultLong( const BigInt& rB, BigInt& rErg ) const
 {
-    int    i, j;
-    sal_uInt32  nZ, k;
+    int        i, j;
+    sal_uInt32 nZ, k;
 
     rErg.bIsNeg = bIsNeg != rB.bIsNeg;
     rErg.bIsBig = true;
@@ -310,11 +311,11 @@ void BigInt::DivLong( const BigInt& rB, BigInt& rErg ) const
 {
     int    i, j;
     sal_uInt16 nK, nQ, nMult;
-    short  nLenB  = rB.nLen;
-    short  nLenB1 = rB.nLen - 1;
+    sal_uInt16  nLenB  = rB.nLen;
+    sal_uInt16  nLenB1 = rB.nLen - 1;
     BigInt aTmpA, aTmpB;
 
-    nMult = (sal_uInt16)(0x10000L / ((long)rB.nNum[nLenB1] + 1));
+    nMult = (sal_uInt16)(0x10000L / ((sal_Int32)rB.nNum[nLenB1] + 1));
 
     aTmpA.Mult( *this, nMult );
     if ( aTmpA.nLen == nLen )
@@ -327,7 +328,7 @@ void BigInt::DivLong( const BigInt& rB, BigInt& rErg ) const
 
     for (j = aTmpA.nLen - 1; j >= nLenB; j--)
     { // guess divisor
-        long nTmp = ( (long)aTmpA.nNum[j] << 16 ) + aTmpA.nNum[j - 1];
+        sal_Int32 nTmp = ( (sal_Int32)aTmpA.nNum[j] << 16 ) + aTmpA.nNum[j - 1];
         if (aTmpA.nNum[j] == aTmpB.nNum[nLenB1])
             nQ = 0xFFFF;
         else
@@ -340,15 +341,15 @@ void BigInt::DivLong( const BigInt& rB, BigInt& rErg ) const
         nK = 0;
         for (i = 0; i < nLenB; i++)
         {
-            nTmp = (long)aTmpA.nNum[j - nLenB + i]
-                   - ((long)aTmpB.nNum[i] * nQ)
+            nTmp = (sal_Int32)aTmpA.nNum[j - nLenB + i]
+                   - ((sal_Int32)aTmpB.nNum[i] * nQ)
                    - nK;
             aTmpA.nNum[j - nLenB + i] = (sal_uInt16)nTmp;
             nK = (sal_uInt16) (nTmp >> 16);
             if ( nK )
                 nK = (sal_uInt16)(0x10000UL - nK);
         }
-        unsigned short& rNum( aTmpA.nNum[j - nLenB + i] );
+        sal_uInt16& rNum( aTmpA.nNum[j - nLenB + i] );
         rNum -= nK;
         if (aTmpA.nNum[j - nLenB + i] == 0)
             rErg.nNum[j - nLenB] = nQ;
@@ -375,13 +376,13 @@ void BigInt::DivLong( const BigInt& rB, BigInt& rErg ) const
 
 void BigInt::ModLong( const BigInt& rB, BigInt& rErg ) const
 {
-    short  i, j;
+    sal_uInt16 i, j;
     sal_uInt16 nK, nQ, nMult;
-    short  nLenB  = rB.nLen;
-    short  nLenB1 = rB.nLen - 1;
+    sal_Int16  nLenB  = rB.nLen;
+    sal_Int16  nLenB1 = rB.nLen - 1;
     BigInt aTmpA, aTmpB;
 
-    nMult = (sal_uInt16)(0x10000L / ((long)rB.nNum[nLenB1] + 1));
+    nMult = (sal_uInt16)(0x10000L / ((sal_Int32)rB.nNum[nLenB1] + 1));
 
     aTmpA.Mult( *this, nMult);
     if ( aTmpA.nLen == nLen )
@@ -394,7 +395,7 @@ void BigInt::ModLong( const BigInt& rB, BigInt& rErg ) const
 
     for (j = aTmpA.nLen - 1; j >= nLenB; j--)
     { // Guess divisor
-        long nTmp = ( (long)aTmpA.nNum[j] << 16 ) + aTmpA.nNum[j - 1];
+        sal_Int32 nTmp = ( (sal_Int32)aTmpA.nNum[j] << 16 ) + aTmpA.nNum[j - 1];
         if (aTmpA.nNum[j] == aTmpB.nNum[nLenB1])
             nQ = 0xFFFF;
         else
@@ -407,15 +408,15 @@ void BigInt::ModLong( const BigInt& rB, BigInt& rErg ) const
         nK = 0;
         for (i = 0; i < nLenB; i++)
         {
-            nTmp = (long)aTmpA.nNum[j - nLenB + i]
-                   - ((long)aTmpB.nNum[i] * nQ)
+            nTmp = (sal_Int32)aTmpA.nNum[j - nLenB + i]
+                   - ((sal_Int32)aTmpB.nNum[i] * nQ)
                    - nK;
             aTmpA.nNum[j - nLenB + i] = (sal_uInt16)nTmp;
             nK = (sal_uInt16) (nTmp >> 16);
             if ( nK )
                 nK = (sal_uInt16)(0x10000UL - nK);
         }
-        unsigned short& rNum( aTmpA.nNum[j - nLenB + i] );
+        sal_uInt16& rNum( aTmpA.nNum[j - nLenB + i] );
         rNum = rNum - nK;
         if (aTmpA.nNum[j - nLenB + i] == 0)
             rErg.nNum[j - nLenB] = nQ;
@@ -574,24 +575,23 @@ BigInt::BigInt( sal_uInt32 nValue )
     }
 }
 
-#if SAL_TYPES_SIZEOFLONG < SAL_TYPES_SIZEOFLONGLONG
-BigInt::BigInt( long long nValue )
+BigInt::BigInt( sal_Int64 nValue )
     : nVal(0)
 {
     bIsSet = true;
     bIsNeg = nValue < 0;
     nLen = 0;
 
-    if ((nValue >= std::numeric_limits<long>::min()) && (nValue <= std::numeric_limits<long>::max()))
+    if ((nValue >= SAL_MIN_INT32) && (nValue <= SAL_MAX_INT32))
     {
         bIsBig = false;
-        nVal   = static_cast<long>(nValue);
+        nVal   = static_cast<sal_Int32>(nValue);
     }
     else
     {
         bIsBig  = true;
-        unsigned long long nUValue = static_cast<unsigned long long>(bIsNeg ? -nValue : nValue);
-        for (int i = 0; (i != sizeof(unsigned long long) / 2) && (nUValue != 0); ++i)
+        sal_uInt64 nUValue = static_cast<sal_uInt64>(bIsNeg ? -nValue : nValue);
+        for (int i = 0; (i != sizeof(sal_uInt64) / 2) && (nUValue != 0); ++i)
         {
             nNum[i] = static_cast<sal_uInt16>(nUValue & 0xffffUL);
             nUValue = nUValue >> 16;
@@ -599,21 +599,6 @@ BigInt::BigInt( long long nValue )
         }
     }
 }
-#endif
-
-BigInt::operator sal_uIntPtr() const
-{
-    if ( !bIsBig )
-        return (sal_uInt32)nVal;
-    else if ( nLen == 2 )
-    {
-        sal_uInt32 nRet;
-        nRet  = ((sal_uInt32)nNum[1]) << 16;
-        nRet += nNum[0];
-        return nRet;
-    }
-    return 0;
-}
 
 BigInt::operator double() const
 {
@@ -708,6 +693,9 @@ BigInt& BigInt::operator-=( const BigInt& rVal )
 
 BigInt& BigInt::operator*=( const BigInt& rVal )
 {
+    static const sal_Int32 MY_MAXSHORT = 0x00007fff;
+    static const sal_Int32 MY_MINSHORT = -MY_MAXSHORT;
+
     if ( !bIsBig && !rVal.bIsBig
          && nVal <= MY_MAXSHORT && rVal.nVal <= MY_MAXSHORT
          && nVal >= MY_MINSHORT && rVal.nVal >= MY_MINSHORT )
@@ -752,7 +740,7 @@ BigInt& BigInt::operator/=( const BigInt& rVal )
             return *this;
         }
 
-        if ( rVal.nVal <= (long)0xFFFF && rVal.nVal >= -(long)0xFFFF )
+        if ( rVal.nVal <= 0xFFFF && rVal.nVal >= -0xFFFF )
         {
             // Divide BigInt with an sal_uInt16
             sal_uInt16 nTmp;
@@ -772,7 +760,7 @@ BigInt& BigInt::operator/=( const BigInt& rVal )
 
     if ( ABS_IsLess( rVal ) )
     {
-        *this = BigInt( (long)0 );
+        *this = BigInt( 0 );
         return *this;
     }
 
@@ -802,9 +790,9 @@ BigInt& BigInt::operator%=( const BigInt& rVal )
             return *this;
         }
 
-        if ( rVal.nVal <= (long)0xFFFF && rVal.nVal >= -(long)0xFFFF )
+        if ( rVal.nVal <= 0xFFFF && rVal.nVal >= -0xFFFF )
         {
-            // Divide Bigint by short
+            // Divide Bigint by int16
             sal_uInt16 nTmp;
             if ( rVal.nVal < 0 )
             {
@@ -815,7 +803,7 @@ BigInt& BigInt::operator%=( const BigInt& rVal )
                 nTmp = (sal_uInt16) rVal.nVal;
 
             Div( nTmp, nTmp );
-            *this = BigInt( (long)nTmp );
+            *this = BigInt( nTmp );
             return *this;
         }
     }