[Libreoffice-commits] .: 3 commits - svtools/source tools/inc tools/qa tools/source

Caolán McNamara caolan at kemper.freedesktop.org
Fri Jul 29 02:57:48 PDT 2011


 svtools/source/filter/wmf/winwmf.cxx  |   62 ++++++++++--------
 svtools/source/filter/wmf/wmf.cxx     |   17 ++++-
 tools/inc/tools/globname.hxx          |   19 -----
 tools/inc/tools/stream.hxx            |   18 +++--
 tools/qa/cppunit/test_streamstate.cxx |    5 +
 tools/source/ref/globname.cxx         |   59 -----------------
 tools/source/stream/stream.cxx        |  113 ++++++++++++++++++++++------------
 7 files changed, 141 insertions(+), 152 deletions(-)

New commits:
commit 1d9a709e3002146a8c80f768f60562193bdc4eb9
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Fri Jul 29 10:53:34 2011 +0100

    check that reads succeeded

diff --git a/svtools/source/filter/wmf/winwmf.cxx b/svtools/source/filter/wmf/winwmf.cxx
index a3ad407..e29ac5a 100644
--- a/svtools/source/filter/wmf/winwmf.cxx
+++ b/svtools/source/filter/wmf/winwmf.cxx
@@ -1000,14 +1000,17 @@ void WMFReader::ReadRecordParams( sal_uInt16 nFunc )
 
 sal_Bool WMFReader::ReadHeader(WMF_APMFILEHEADER *pAPMHeader)
 {
-    Rectangle	aPlaceableBound;
-    sal_uInt32  nl, nStrmPos = pWMF->Tell();
+    sal_Size nStrmPos = pWMF->Tell();
 
+    sal_uInt32 nPlaceableMetaKey(0);
     // Einlesen des METAFILEHEADER, falls vorhanden
-    *pWMF >> nl;
+    *pWMF >> nPlaceableMetaKey;
+    if (!pWMF->good())
+        return false;
 
-    Size aWMFSize;
-    if ( nl == 0x9ac6cdd7L )
+    Rectangle aPlaceableBound;
+
+    if (nPlaceableMetaKey == 0x9ac6cdd7L)
     {
         sal_Int16 nVal;
 
@@ -1015,10 +1018,14 @@ sal_Bool WMFReader::ReadHeader(WMF_APMFILEHEADER *pAPMHeader)
         pWMF->SeekRel(2);
 
         // BoundRect
-        *pWMF >> nVal; aPlaceableBound.Left() = nVal;
-        *pWMF >> nVal; aPlaceableBound.Top() = nVal;
-        *pWMF >> nVal; aPlaceableBound.Right() = nVal;
-        *pWMF >> nVal; aPlaceableBound.Bottom() = nVal;
+        *pWMF >> nVal;
+        aPlaceableBound.Left() = nVal;
+        *pWMF >> nVal;
+        aPlaceableBound.Top() = nVal;
+        *pWMF >> nVal;
+        aPlaceableBound.Right() = nVal;
+        *pWMF >> nVal;
+        aPlaceableBound.Bottom() = nVal;
 
         // inch
         *pWMF >> nUnitsPerInch;
@@ -1031,22 +1038,23 @@ sal_Bool WMFReader::ReadHeader(WMF_APMFILEHEADER *pAPMHeader)
     }
     else
     {
-      nUnitsPerInch = (pAPMHeader!=NULL?pAPMHeader->inch:96);
-      pWMF->Seek( nStrmPos + 18 );    // set the streampos to the start of the the metaactions
-      GetPlaceableBound( aPlaceableBound, pWMF );
-      pWMF->Seek( nStrmPos );
-      if (pAPMHeader!=NULL) {
-        // #n417818#: If we have an external header then overwrite the bounds!
-        aPlaceableBound=Rectangle(pAPMHeader->left*567*nUnitsPerInch/1440/1000,
-                      pAPMHeader->top*567*nUnitsPerInch/1440/1000,
-                      pAPMHeader->right*567*nUnitsPerInch/1440/1000,
-                      pAPMHeader->bottom*567*nUnitsPerInch/1440/1000);
-      }
+        nUnitsPerInch = (pAPMHeader!=NULL?pAPMHeader->inch:96);
+        pWMF->Seek( nStrmPos + 18 );    // set the streampos to the start of the the metaactions
+        GetPlaceableBound( aPlaceableBound, pWMF );
+        pWMF->Seek( nStrmPos );
+        if (pAPMHeader!=NULL)
+        {
+            // #n417818#: If we have an external header then overwrite the bounds!
+            aPlaceableBound=Rectangle(pAPMHeader->left*567*nUnitsPerInch/1440/1000,
+                          pAPMHeader->top*567*nUnitsPerInch/1440/1000,
+                          pAPMHeader->right*567*nUnitsPerInch/1440/1000,
+                          pAPMHeader->bottom*567*nUnitsPerInch/1440/1000);
+        }
     }
 
     pOut->SetUnitsPerInch( nUnitsPerInch );
     pOut->SetWinOrg( aPlaceableBound.TopLeft() );
-    aWMFSize = Size( labs( aPlaceableBound.GetWidth() ), labs( aPlaceableBound.GetHeight() ) );
+    Size aWMFSize( labs( aPlaceableBound.GetWidth() ), labs( aPlaceableBound.GetHeight() ) );
     pOut->SetWinExt( aWMFSize );
 
     Size aDevExt( 10000, 10000 );
@@ -1060,12 +1068,14 @@ sal_Bool WMFReader::ReadHeader(WMF_APMFILEHEADER *pAPMHeader)
     pOut->SetDevExt( aDevExt );
 
     // Einlesen des METAHEADER
-    *pWMF >> nl; // Typ und Headergroesse
-
-    if( nl != 0x00090001 )
+    sal_uInt32 nMetaKey(0);
+    *pWMF >> nMetaKey; // Typ und Headergroesse
+    if (!pWMF->good())
+        return false;
+    if (nMetaKey != 0x00090001)
     {
         pWMF->SetError( SVSTREAM_FILEFORMAT_ERROR );
-        return sal_False;
+        return false;
     }
 
     pWMF->SeekRel( 2 ); // Version (von Windows)
@@ -1074,7 +1084,7 @@ sal_Bool WMFReader::ReadHeader(WMF_APMFILEHEADER *pAPMHeader)
     pWMF->SeekRel( 4 ); // MaxRecord (Groesse des groessten Records in Words)
     pWMF->SeekRel( 2 ); // NoParameters (Unused
 
-    return sal_True;
+    return pWMF->good();
 }
 
 void WMFReader::ReadWMF(WMF_APMFILEHEADER *pAPMHeader)
diff --git a/svtools/source/filter/wmf/wmf.cxx b/svtools/source/filter/wmf/wmf.cxx
index 03923dd..479ec5b 100644
--- a/svtools/source/filter/wmf/wmf.cxx
+++ b/svtools/source/filter/wmf/wmf.cxx
@@ -33,6 +33,7 @@
 #include "emfwr.hxx"
 #include "wmfwr.hxx"
 #include <svtools/wmf.hxx>
+#include <comphelper/scopeguard.hxx>
 
 // -----------------------------------------------------------------------------
 
@@ -62,13 +63,23 @@ sal_Bool ConvertWMFToGDIMetaFile( SvStream & rStreamWMF, GDIMetaFile & rGDIMetaF
 
 sal_Bool ReadWindowMetafile( SvStream& rStream, GDIMetaFile& rMTF, FilterConfigItem* pFilterConfigItem )
 {
-    sal_uInt32 nMetaType;
+    sal_uInt32 nMetaType(0);
     sal_uInt32 nOrgPos = rStream.Tell();
+
     sal_uInt16 nOrigNumberFormat = rStream.GetNumberFormatInt();
     rStream.SetNumberFormatInt( NUMBERFORMAT_INT_LITTLEENDIAN );
+    //exception-safe reset nOrigNumberFormat at end of scope
+    const ::comphelper::ScopeGuard aScopeGuard(
+        boost::bind(&SvStream::SetNumberFormatInt, ::boost::ref(rStream),
+          nOrigNumberFormat));
+
     rStream.Seek( 0x28 );
     rStream >> nMetaType;
     rStream.Seek( nOrgPos );
+
+    if (!rStream.good())
+        return false;
+
     if ( nMetaType == 0x464d4520 )
     {
         if ( EnhWMFReader( rStream, rMTF, NULL ).ReadEnhWMF() == sal_False )
@@ -78,8 +89,8 @@ sal_Bool ReadWindowMetafile( SvStream& rStream, GDIMetaFile& rMTF, FilterConfigI
     {
         WMFReader( rStream, rMTF, pFilterConfigItem ).ReadWMF();
     }
-    rStream.SetNumberFormatInt( nOrigNumberFormat );
-    return !rStream.GetError();
+
+    return rStream.good();
 }
 
 // -----------------------------------------------------------------------------
commit eaa3237c5f433c74e74cbde718c3f82008d0058c
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Fri Jul 29 00:02:42 2011 +0100

    make stream operators leave variables in original state on failure
    
    this aligns them with the behaviour of std::stream, and makes things
    like
    
    sal_uInt32 n(0);
    rSt >> n;
    if (n)
    {
       ...
    }
    
    safe if there was a short read of e.g. 3 bytes instead of the required 4

diff --git a/tools/inc/tools/stream.hxx b/tools/inc/tools/stream.hxx
index fcc4a95..c97cf77 100644
--- a/tools/inc/tools/stream.hxx
+++ b/tools/inc/tools/stream.hxx
@@ -526,18 +526,22 @@ public:
 
     friend SvStream& operator<<( SvStream& rStr, SvStrPtr f ); // fuer Manips
 
-    //end of input seen
+    //end of input seen during previous i/o operation
     bool eof() const { return bIsEof; }
 
     // stream is broken
     bool bad() const { return GetError() != 0; }
 
-    //next operation might succeed. If the state is good() the previous i/o
-    //operation succeeded. If the state is good(), the next input operation
-    //might succeed; otherwise, it will fail. Applying an input operation to a
-    //stream that is not in the good() state is a null operation as far as the
-    //variable being read into is concerned. If we try to read into a variable
-    //v and the operation fails, the value of v should be unchanged,
+    //If the state is good() the previous i/o operation succeeded.
+    //
+    //If the state is good(), the next input operation might succeed;
+    //otherwise, it will fail.
+    //
+    //Applying an input operation to a stream that is not in the good() state
+    //is a null operation as far as the variable being read into is concerned.
+    //
+    //If we try to read into a variable v and the operation fails, the value of
+    //v should be unchanged,
     bool good() const { return !(eof() || bad()); }
 };
 
diff --git a/tools/qa/cppunit/test_streamstate.cxx b/tools/qa/cppunit/test_streamstate.cxx
index c2a4be0..182ad6c 100644
--- a/tools/qa/cppunit/test_streamstate.cxx
+++ b/tools/qa/cppunit/test_streamstate.cxx
@@ -94,6 +94,9 @@ namespace
         //yet, the read didn't succeed
         CPPUNIT_ASSERT(!aMemStream.good());
 
+        //set things up so that there is only one byte available on an attempt
+        //to read a two-byte sal_uInt16.  The byte should be consumed, but the
+        //operation should fail, and tools_b should remain unchanged,
         sal_uInt16 tools_b = 0x1122;
         aMemStream.SeekRel(-1);
         CPPUNIT_ASSERT(!aMemStream.eof());
@@ -101,7 +104,7 @@ namespace
         aMemStream >> tools_b;
         CPPUNIT_ASSERT(!aMemStream.good());
         CPPUNIT_ASSERT(aMemStream.eof());
-//        CPPUNIT_ASSERT(tools_b == 0x1122); //nasty, real nasty
+        CPPUNIT_ASSERT(tools_b == 0x1122);
 
         iss.clear();
         iss.seekg(0);
diff --git a/tools/source/stream/stream.cxx b/tools/source/stream/stream.cxx
index ab2698f..1a3f406 100644
--- a/tools/source/stream/stream.cxx
+++ b/tools/source/stream/stream.cxx
@@ -1158,58 +1158,89 @@ sal_Size SvStream::SeekRel( sal_sSize nPos )
 |*
 *************************************************************************/
 
-SvStream& SvStream::operator >> ( sal_uInt16& r )
+SvStream& SvStream::operator>>(sal_uInt16& r)
 {
-    READNUMBER_WITHOUT_SWAP(sal_uInt16,r)
-    if( bSwap )
-        SwapUShort(r);
+    sal_uInt16 n;
+    READNUMBER_WITHOUT_SWAP(sal_uInt16, n)
+    if (good())
+    {
+        if (bSwap)
+            SwapUShort(n);
+        r = n;
+    }
     return *this;
 }
 
-SvStream& SvStream::operator>> ( sal_uInt32& r )
+SvStream& SvStream::operator>>(sal_uInt32& r)
 {
-    READNUMBER_WITHOUT_SWAP(sal_uInt32,r)
-    if( bSwap )
-        SwapULong(r);
+    sal_uInt32 n;
+    READNUMBER_WITHOUT_SWAP(sal_uInt32, n)
+    if (good())
+    {
+        if (bSwap)
+            SwapULong(n);
+        r = n;
+    }
     return *this;
 }
 
 
-SvStream& SvStream::operator>> ( sal_uInt64& r )
+SvStream& SvStream::operator>>(sal_uInt64& r)
 {
-    READNUMBER_WITHOUT_SWAP(sal_uInt64,r)
-    if( bSwap )
-        SwapUInt64(r);
+    sal_uInt64 n;
+    READNUMBER_WITHOUT_SWAP(sal_uInt64, n)
+    if (good())
+    {
+        if (bSwap)
+            SwapUInt64(n);
+        r = n;
+    }
     return *this;
 }
 
-SvStream& SvStream::operator >> ( long& r )
+SvStream& SvStream::operator >>(long& r) //puke!, kill this
 {
 #if(SAL_TYPES_SIZEOFLONG != 4)
-    int tmp = r;
-    *this >> tmp;
-    r = tmp;
+    int n;
+    *this >> n;
+    if (good())
+        r = n;
 #else
-    READNUMBER_WITHOUT_SWAP(long,r)
-    if( bSwap )
-        SwapLong(r);
+    long n;
+    READNUMBER_WITHOUT_SWAP(long, n)
+    if (good())
+    {
+        if (bSwap)
+            SwapLong(n);
+        r = n;
+    }
 #endif
     return *this;
 }
 
-SvStream& SvStream::operator >> ( short& r )
+SvStream& SvStream::operator>>(short& r)
 {
-    READNUMBER_WITHOUT_SWAP(short,r)
-    if( bSwap )
-        SwapShort(r);
+    short n;
+    READNUMBER_WITHOUT_SWAP(short, n)
+    if (good())
+    {
+        if(bSwap)
+            SwapShort(n);
+        r = n;
+    }
     return *this;
 }
 
-SvStream& SvStream::operator >> ( int& r )
+SvStream& SvStream::operator>>(int& r)
 {
-    READNUMBER_WITHOUT_SWAP(int,r)
-    if( bSwap )
-        SwapLongInt(r);
+    int n;
+    READNUMBER_WITHOUT_SWAP(int, n)
+    if (good())
+    {
+        if (bSwap)
+            SwapLongInt(n);
+        r = n;
+    }
     return *this;
 }
 
@@ -1260,25 +1291,33 @@ SvStream& SvStream::operator>>( unsigned char& r )
     return *this;
 }
 
-SvStream& SvStream::operator>>( float& r )
+SvStream& SvStream::operator>>(float& r)
 {
-    // Read( (char*)&r, sizeof(float) );
-    READNUMBER_WITHOUT_SWAP(float,r)
+    float n;
+    READNUMBER_WITHOUT_SWAP(float, n)
+    if (good())
+    {
 #if defined UNX
-    if( bSwap )
-      SwapFloat(r);
+        if (bSwap)
+          SwapFloat(n);
 #endif
+        r = n;
+    }
     return *this;
 }
 
-SvStream& SvStream::operator>>( double& r )
+SvStream& SvStream::operator>>(double& r)
 {
-    // Read( (char*)&r, sizeof(double) );
-    READNUMBER_WITHOUT_SWAP(double,r)
+    double n;
+    READNUMBER_WITHOUT_SWAP(double, n)
+    if (good())
+    {
 #if defined UNX
-    if( bSwap )
-      SwapDouble(r);
+        if (bSwap)
+          SwapDouble(n);
+        r = n;
 #endif
+    }
     return *this;
 }
 
commit f7612869f3b767c6d0ac58fbe28448b66b8c98ed
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Thu Jul 28 22:47:00 2011 +0100

    callcatcher: ditch various unused methods

diff --git a/tools/inc/tools/globname.hxx b/tools/inc/tools/globname.hxx
index 552b702..c6587cc 100644
--- a/tools/inc/tools/globname.hxx
+++ b/tools/inc/tools/globname.hxx
@@ -59,10 +59,8 @@ struct GUID;
 #endif
 typedef GUID CLSID;
 class SvStream;
-class SvGlobalNameList;
 class TOOLS_DLLPUBLIC SvGlobalName
 {
-friend class SvGlobalNameList;
     ImpSvGlobalName * pImp;
     void    NewImp();
 public:
@@ -119,23 +117,6 @@ public:
     com::sun::star::uno::Sequence < sal_Int8 > GetByteSequence() const;
 };
 
-class SvGlobalNameList
-{
-    std::vector<ImpSvGlobalName*> aList;
-public:
-                    SvGlobalNameList();
-                    ~SvGlobalNameList();
-
-    void            Append( const SvGlobalName & );
-    SvGlobalName    GetObject( sal_uLong );
-    sal_Bool            IsEntry( const SvGlobalName & rName );
-    size_t          Count() const { return aList.size(); }
-private:
-                // nicht erlaubt
-                SvGlobalNameList( const SvGlobalNameList & );
-    SvGlobalNameList & operator = ( const SvGlobalNameList & );
-};
-
 #endif // _GLOBNAME_HXX
 
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/tools/source/ref/globname.cxx b/tools/source/ref/globname.cxx
index 06b7ba7..5f4f6bd 100644
--- a/tools/source/ref/globname.cxx
+++ b/tools/source/ref/globname.cxx
@@ -406,65 +406,6 @@ String SvGlobalName::GetHexName() const
     return rtl::OStringToOUString(aHexBuffer.makeStringAndClear(), RTL_TEXTENCODING_ASCII_US);
 }
 
-/************** SvGlobalNameList ****************************************/
-/************************************************************************/
-/*************************************************************************
-|*	  SvGlobalNameList::SvGlobalNameList()
-*************************************************************************/
-SvGlobalNameList::SvGlobalNameList()
-{
-}
-
-/*************************************************************************
-|*	  SvGlobalNameList::~SvGlobalNameList()
-*************************************************************************/
-SvGlobalNameList::~SvGlobalNameList()
-{
-    ImpSvGlobalName *pImp = 0;
-    std::vector<ImpSvGlobalName*>::iterator piter;
-
-    for (piter = aList.begin(); piter != aList.end(); ++piter)
-    {
-        pImp = *piter;
-
-        --pImp->nRefCount;
-        if( !pImp->nRefCount )
-            delete pImp;
-    }
-}
-
-/*************************************************************************
-|*	  SvGlobalNameList::Append()
-*************************************************************************/
-void SvGlobalNameList::Append( const SvGlobalName & rName )
-{
-    rName.pImp->nRefCount++;
-    aList.push_back(rName.pImp);
-}
-
-/*************************************************************************
-|*	  SvGlobalNameList::GetObject()
-*************************************************************************/
-SvGlobalName SvGlobalNameList::GetObject( sal_uLong nPos )
-{
-    return SvGlobalName(nPos < aList.size() ? aList[nPos] : NULL);
-}
-
-/*************************************************************************
-|*	  SvGlobalNameList::IsEntry()
-*************************************************************************/
-sal_Bool SvGlobalNameList::IsEntry( const SvGlobalName & rName )
-{
-    std::vector<ImpSvGlobalName*>::iterator piter;
-    for (piter = aList.begin(); piter != aList.end(); ++piter)
-    {
-        if (*rName.pImp == *(*piter))
-            return sal_True;
-    }
-
-    return sal_False;
-}
-
 com::sun::star::uno::Sequence < sal_Int8 > SvGlobalName::GetByteSequence() const
 {
     // platform independent representation of a "GlobalName"


More information about the Libreoffice-commits mailing list