[Libreoffice-commits] core.git: include/tools sd/source svx/source tools/qa tools/source
Noel Grandin
noel.grandin at collabora.co.uk
Fri Sep 15 07:54:21 UTC 2017
include/tools/fract.hxx | 13 -
sd/source/ui/unoidl/UnoDocumentSettings.cxx | 4
sd/source/ui/view/frmview.cxx | 8
svx/source/customshapes/EnhancedCustomShapeFunctionParser.cxx | 2
tools/qa/cppunit/test_fract.cxx | 19 --
tools/source/generic/fract.cxx | 88 +++-------
6 files changed, 47 insertions(+), 87 deletions(-)
New commits:
commit 331e2e5ed3bf4e0b2c1fab3b7bca836170317827
Author: Noel Grandin <noel.grandin at collabora.co.uk>
Date: Thu Sep 14 08:49:52 2017 +0200
long->sal_Int32 in Fraction
because long is 32bits on Windows and 64bits on Linux.
Reasoning:
(a) all the users of Fraction used to be 32bit in the past
(b) this makes the Linux code behave the same as the Windows code
(c) changing it to 64bits would be dangerous because then call sites
could see silent truncation.
Change-Id: I2a36200623a3cf2e7d62ccad030a20614e1798fb
Reviewed-on: https://gerrit.libreoffice.org/42200
Tested-by: Jenkins <ci at libreoffice.org>
Reviewed-by: Noel Grandin <noel.grandin at collabora.co.uk>
diff --git a/include/tools/fract.hxx b/include/tools/fract.hxx
index 4cde7db061db..68c0984a19a5 100644
--- a/include/tools/fract.hxx
+++ b/include/tools/fract.hxx
@@ -25,8 +25,6 @@
class SvStream;
-// This class uses the platform defined type 'long' as valid values but do all
-// calculations using sal_Int64 with checks for 'long' overflows.
class SAL_WARN_UNUSED TOOLS_DLLPUBLIC Fraction final
{
struct Impl;
@@ -39,16 +37,19 @@ public:
Fraction();
Fraction( const Fraction & rFrac );
Fraction( Fraction && rFrac );
- Fraction( long nNum, long nDen );
+ Fraction( sal_Int64 nNum, sal_Int64 nDen );
explicit Fraction( double dVal );
~Fraction();
bool IsValid() const;
- long GetNumerator() const;
- long GetDenominator() const;
+ sal_Int32 GetNumerator() const;
+ sal_Int32 GetDenominator() const;
- explicit operator long() const;
+ explicit operator sal_Int32() const;
+#if SAL_TYPES_SIZEOFLONG == 8
+ explicit operator long() const { return sal_Int32(*this); }
+#endif
explicit operator double() const;
Fraction& operator=( const Fraction& rfrFrac );
diff --git a/sd/source/ui/unoidl/UnoDocumentSettings.cxx b/sd/source/ui/unoidl/UnoDocumentSettings.cxx
index cbe2aa9ddb1b..01b09b80623b 100644
--- a/sd/source/ui/unoidl/UnoDocumentSettings.cxx
+++ b/sd/source/ui/unoidl/UnoDocumentSettings.cxx
@@ -1083,10 +1083,10 @@ DocumentSettings::_getPropertyValues(
}
break;
case HANDLE_SCALE_NUM:
- *pValue <<= (sal_Int32)pDoc->GetUIScale().GetNumerator();
+ *pValue <<= pDoc->GetUIScale().GetNumerator();
break;
case HANDLE_SCALE_DOM:
- *pValue <<= (sal_Int32)pDoc->GetUIScale().GetDenominator();
+ *pValue <<= pDoc->GetUIScale().GetDenominator();
break;
case HANDLE_TABSTOP:
*pValue <<= (sal_Int32)pDoc->GetDefaultTabulator();
diff --git a/sd/source/ui/view/frmview.cxx b/sd/source/ui/view/frmview.cxx
index e96d9cc45299..4031b0ac1e20 100644
--- a/sd/source/ui/view/frmview.cxx
+++ b/sd/source/ui/view/frmview.cxx
@@ -437,10 +437,10 @@ void FrameView::WriteUserDataSequence ( css::uno::Sequence < css::beans::Propert
aUserData.addValue( sUNO_View_GridCoarseHeight, makeAny( (sal_Int32)GetGridCoarse().Height() ) );
aUserData.addValue( sUNO_View_GridFineWidth, makeAny( (sal_Int32)GetGridFine().Width() ) );
aUserData.addValue( sUNO_View_GridFineHeight, makeAny( (sal_Int32)GetGridFine().Height() ) );
- aUserData.addValue( sUNO_View_GridSnapWidthXNumerator, makeAny( (sal_Int32)GetSnapGridWidthX().GetNumerator() ) );
- aUserData.addValue( sUNO_View_GridSnapWidthXDenominator, makeAny( (sal_Int32)GetSnapGridWidthX().GetDenominator() ) );
- aUserData.addValue( sUNO_View_GridSnapWidthYNumerator, makeAny( (sal_Int32)GetSnapGridWidthY().GetNumerator() ) );
- aUserData.addValue( sUNO_View_GridSnapWidthYDenominator, makeAny( (sal_Int32)GetSnapGridWidthY().GetDenominator() ) );
+ aUserData.addValue( sUNO_View_GridSnapWidthXNumerator, makeAny( GetSnapGridWidthX().GetNumerator() ) );
+ aUserData.addValue( sUNO_View_GridSnapWidthXDenominator, makeAny( GetSnapGridWidthX().GetDenominator() ) );
+ aUserData.addValue( sUNO_View_GridSnapWidthYNumerator, makeAny( GetSnapGridWidthY().GetNumerator() ) );
+ aUserData.addValue( sUNO_View_GridSnapWidthYDenominator, makeAny( GetSnapGridWidthY().GetDenominator() ) );
aUserData.addValue( sUNO_View_IsAngleSnapEnabled, makeAny( IsAngleSnapEnabled() ) );
aUserData.addValue( sUNO_View_SnapAngle, makeAny( (sal_Int32)GetSnapAngle() ) );
diff --git a/svx/source/customshapes/EnhancedCustomShapeFunctionParser.cxx b/svx/source/customshapes/EnhancedCustomShapeFunctionParser.cxx
index a97d6e0b9f03..515d249b4bdb 100644
--- a/svx/source/customshapes/EnhancedCustomShapeFunctionParser.cxx
+++ b/svx/source/customshapes/EnhancedCustomShapeFunctionParser.cxx
@@ -117,7 +117,7 @@ public:
if ( aFract.GetDenominator() == 1 )
{
aRet.Type = EnhancedCustomShapeParameterType::NORMAL;
- aRet.Value <<= (sal_Int32)aFract.GetNumerator();
+ aRet.Value <<= aFract.GetNumerator();
}
else
{
diff --git a/tools/qa/cppunit/test_fract.cxx b/tools/qa/cppunit/test_fract.cxx
index 6352588ef6b2..12977b5635f8 100644
--- a/tools/qa/cppunit/test_fract.cxx
+++ b/tools/qa/cppunit/test_fract.cxx
@@ -73,27 +73,12 @@ public:
aFract3.GetNumerator() == 2 &&
aFract3.GetDenominator() == 1 );
-#if SAL_TYPES_SIZEOFLONG == 8
- Fraction aFract4(0x7AAAAAAAAAAAAAAA, 0x3555555555555555);
- CPPUNIT_ASSERT_MESSAGE( "Fraction #4 cancellation wrong",
- aFract4.GetNumerator() == 0x7AAAAAAAAAAAAAAA &&
- aFract4.GetDenominator() == 0x3555555555555555 );
- aFract4.ReduceInaccurate(62);
- CPPUNIT_ASSERT_MESSAGE( "Fraction #4 ReduceInaccurate errorneously cut precision",
- aFract4.GetNumerator() == 0x7AAAAAAAAAAAAAAA &&
- aFract4.GetDenominator() == 0x3555555555555555 );
-
- aFract4.ReduceInaccurate(61);
- CPPUNIT_ASSERT_MESSAGE( "Fraction #4 ReduceInaccurate reduce to 61 bit failed",
- aFract4.GetNumerator() == 0x3D55555555555555 &&
- aFract4.GetDenominator() == 0x1AAAAAAAAAAAAAAA );
-#endif
}
void testMinLongDouble() {
Fraction f(double(SAL_MIN_INT32));
- CPPUNIT_ASSERT_EQUAL(long(SAL_MIN_INT32), f.GetNumerator());
- CPPUNIT_ASSERT_EQUAL(1L, f.GetDenominator());
+ CPPUNIT_ASSERT_EQUAL(SAL_MIN_INT32, f.GetNumerator());
+ CPPUNIT_ASSERT_EQUAL(sal_Int32(1), f.GetDenominator());
}
void testCreateFromDoubleIn32BitsPlatform() {
diff --git a/tools/source/generic/fract.cxx b/tools/source/generic/fract.cxx
index 7f35dd00907b..799ac4575018 100644
--- a/tools/source/generic/fract.cxx
+++ b/tools/source/generic/fract.cxx
@@ -33,16 +33,14 @@
#include <boost/math/common_factor_rt.hpp>
#include <boost/rational.hpp>
-template<typename T>
-static boost::rational<T> rational_FromDouble(double dVal);
+static boost::rational<sal_Int32> rational_FromDouble(double dVal);
-template<typename T>
-static void rational_ReduceInaccurate(boost::rational<T>& rRational, unsigned nSignificantBits);
+static void rational_ReduceInaccurate(boost::rational<sal_Int32>& rRational, unsigned nSignificantBits);
struct Fraction::Impl
{
bool valid;
- boost::rational<sal_Int64> value;
+ boost::rational<sal_Int32> value;
Impl()
: valid(false)
@@ -72,8 +70,12 @@ Fraction::Fraction( Fraction&& rFrac ) : mpImpl(std::move(rFrac.mpImpl))
// Negative values in the denominator are invalid and cause the
// inversion of both nominator and denominator signs
// in order to return the correct value.
-Fraction::Fraction( long nNum, long nDen ) : mpImpl(new Impl)
+Fraction::Fraction( sal_Int64 nNum, sal_Int64 nDen ) : mpImpl(new Impl)
{
+ assert( nNum >= std::numeric_limits<sal_Int32>::min() );
+ assert( nNum <= std::numeric_limits<sal_Int32>::max( ));
+ assert( nDen >= std::numeric_limits<sal_Int32>::min() );
+ assert( nDen <= std::numeric_limits<sal_Int32>::max( ));
if ( nDen == 0 )
{
mpImpl->valid = false;
@@ -88,7 +90,7 @@ Fraction::Fraction( double dVal ) : mpImpl(new Impl)
{
try
{
- mpImpl->value = rational_FromDouble<sal_Int64>( dVal );
+ mpImpl->value = rational_FromDouble( dVal );
if ( HasOverflowValue() )
throw boost::bad_rational();
mpImpl->valid = true;
@@ -107,10 +109,10 @@ Fraction::~Fraction()
bool Fraction::HasOverflowValue()
{
//coverity[result_independent_of_operands]
- return mpImpl->value.numerator() < std::numeric_limits<long>::min() ||
- mpImpl->value.numerator() > std::numeric_limits<long>::max() ||
- mpImpl->value.denominator() < std::numeric_limits<long>::min() ||
- mpImpl->value.denominator() > std::numeric_limits<long>::max();
+ return mpImpl->value.numerator() < std::numeric_limits<sal_Int32>::min() ||
+ mpImpl->value.numerator() > std::numeric_limits<sal_Int32>::max() ||
+ mpImpl->value.denominator() < std::numeric_limits<sal_Int32>::min() ||
+ mpImpl->value.denominator() > std::numeric_limits<sal_Int32>::max();
}
Fraction::operator double() const
@@ -126,7 +128,7 @@ Fraction::operator double() const
// This methods first validates both values.
// If one of the arguments is invalid, the whole operation is invalid.
-// After computation detect if result overflows a long value
+// After computation detect if result overflows a sal_Int32 value
// which cause the operation to be marked as invalid
Fraction& Fraction::operator += ( const Fraction& rVal )
{
@@ -268,7 +270,7 @@ void Fraction::ReduceInaccurate( unsigned nSignificantBits )
rational_ReduceInaccurate(mpImpl->value, nSignificantBits);
}
-long Fraction::GetNumerator() const
+sal_Int32 Fraction::GetNumerator() const
{
if ( !mpImpl->valid )
{
@@ -278,7 +280,7 @@ long Fraction::GetNumerator() const
return mpImpl->value.numerator();
}
-long Fraction::GetDenominator() const
+sal_Int32 Fraction::GetDenominator() const
{
if ( !mpImpl->valid )
{
@@ -309,14 +311,14 @@ bool Fraction::IsValid() const
return mpImpl->valid;
}
-Fraction::operator long() const
+Fraction::operator sal_Int32() const
{
if ( !mpImpl->valid )
{
- SAL_WARN( "tools.fraction", "'operator long()' on invalid fraction" );
+ SAL_WARN( "tools.fraction", "'operator sal_Int32()' on invalid fraction" );
return 0;
}
- return boost::rational_cast<long>(mpImpl->value);
+ return boost::rational_cast<sal_Int32>(mpImpl->value);
}
Fraction operator+( const Fraction& rVal1, const Fraction& rVal2 )
@@ -421,12 +423,6 @@ SvStream& WriteFraction( SvStream& rOStream, const Fraction& rFract )
rOStream.WriteInt32( 0 );
rOStream.WriteInt32( -1 );
} else {
-#if OSL_DEBUG_LEVEL > 0
- // can only write 32 bits - check that no data is lost!
- boost::rational<sal_Int64> copy(rFract.mpImpl->value);
- rational_ReduceInaccurate(copy, 32);
- assert(copy == rFract.mpImpl->value && "data loss in WriteFraction!");
-#endif
rOStream.WriteInt32( rFract.mpImpl->value.numerator() );
rOStream.WriteInt32( rFract.mpImpl->value.denominator() );
}
@@ -437,21 +433,20 @@ SvStream& WriteFraction( SvStream& rOStream, const Fraction& rFract )
// Otherwise, dVal and denominator are multiplied by 10, until one of them
// is larger than (LONG_MAX / 10).
//
-// NOTE: here we use 'long' due that only values in long range are valid.
-template<typename T>
-static boost::rational<T> rational_FromDouble(double dVal)
+// NOTE: here we use 'sal_Int32' due that only values in sal_Int32 range are valid.
+static boost::rational<sal_Int32> rational_FromDouble(double dVal)
{
- if ( dVal > std::numeric_limits<long>::max() ||
- dVal < std::numeric_limits<long>::min() )
+ if ( dVal > std::numeric_limits<sal_Int32>::max() ||
+ dVal < std::numeric_limits<sal_Int32>::min() )
throw boost::bad_rational();
- const long nMAX = std::numeric_limits<long>::max() / 10;
- long nDen = 1;
+ const sal_Int32 nMAX = std::numeric_limits<sal_Int32>::max() / 10;
+ sal_Int32 nDen = 1;
while ( std::abs( dVal ) < nMAX && nDen < nMAX ) {
dVal *= 10;
nDen *= 10;
}
- return boost::rational<T>( long(dVal), nDen );
+ return boost::rational<sal_Int32>( sal_Int32(dVal), nDen );
}
// Similar to clz_table that can be googled
@@ -463,7 +458,7 @@ const char nbits_table[32] =
21, 12, 16, 8, 11, 7, 6, 5
};
-static int impl_NumberOfBits( unsigned long nNum )
+static int impl_NumberOfBits( sal_uInt32 nNum )
{
// http://en.wikipedia.org/wiki/De_Bruijn_sequence
// background paper: Using de Bruijn Sequences to Index a 1 in a
@@ -485,28 +480,8 @@ static int impl_NumberOfBits( unsigned long nNum )
sal_uInt32 nNumber;
int nBonus;
-#if SAL_TYPES_SIZEOFLONG == 4
nNumber = nNum;
nBonus = 0;
-#elif SAL_TYPES_SIZEOFLONG == 8
- nNum |= ( nNum >> 32 );
-
- if ( nNum & 0x80000000 )
- {
- nNumber = sal_uInt32( nNum >> 32 );
- nBonus = 32;
-
- if ( nNumber == 0 )
- return 32;
- }
- else
- {
- nNumber = sal_uInt32( nNum & 0xFFFFFFFF );
- nBonus = 0;
- }
-#else
-#error "Unknown size of long!"
-#endif
// De facto shift left of nDeBruijn using multiplication (nNumber
// is all ones from topmost bit, thus nDeBruijn + (nDeBruijn *
@@ -539,16 +514,15 @@ static int impl_NumberOfBits( unsigned long nNum )
A ReduceInaccurate(8) yields 1/1.
*/
-template<typename T>
-static void rational_ReduceInaccurate(boost::rational<T>& rRational, unsigned nSignificantBits)
+static void rational_ReduceInaccurate(boost::rational<sal_Int32>& rRational, unsigned nSignificantBits)
{
if ( !rRational )
return;
// http://www.boost.org/doc/libs/release/libs/rational/rational.html#Internal%20representation
const bool bNeg = ( rRational.numerator() < 0 );
- T nMul = bNeg? -rRational.numerator(): rRational.numerator();
- T nDiv = rRational.denominator();
+ sal_Int32 nMul = bNeg? -rRational.numerator(): rRational.numerator();
+ sal_Int32 nDiv = rRational.denominator();
DBG_ASSERT(nSignificantBits<65, "More than 64 bit of significance is overkill!");
@@ -568,7 +542,7 @@ static void rational_ReduceInaccurate(boost::rational<T>& rRational, unsigned nS
return;
}
- rRational.assign( bNeg? -T( nMul ): T( nMul ), nDiv );
+ rRational.assign( bNeg ? -nMul : nMul, nDiv );
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
More information about the Libreoffice-commits
mailing list