[Libreoffice-commits] core.git: 6 commits - cui/source filter/source include/basebmp package/source vcl/inc vcl/opengl vcl/source

Caolán McNamara caolanm at redhat.com
Wed Nov 19 03:01:43 PST 2014


 cui/source/dialogs/SpellDialog.cxx          |    2 
 filter/source/graphicfilter/ipict/ipict.cxx |   12 ++
 include/basebmp/compositeiterator.hxx       |  140 +++++++++++++++++-----------
 package/source/zipapi/ZipFile.cxx           |   13 +-
 vcl/inc/openglgdiimpl.hxx                   |    2 
 vcl/opengl/gdiimpl.cxx                      |   10 --
 vcl/opengl/win/gdiimpl.cxx                  |    4 
 vcl/opengl/x11/gdiimpl.cxx                  |    4 
 vcl/source/control/field.cxx                |    7 +
 9 files changed, 122 insertions(+), 72 deletions(-)

New commits:
commit 5d8e23fdb98a74372ca3da37559af4b45655fee4
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Wed Nov 19 10:43:19 2014 +0000

    coverity#1226488 Big parameter passed by value
    
    Change-Id: I55ee5dcd7ed18fe907fc06aba345d96a90fb2324

diff --git a/include/basebmp/compositeiterator.hxx b/include/basebmp/compositeiterator.hxx
index 4fb9c10..1823181 100644
--- a/include/basebmp/compositeiterator.hxx
+++ b/include/basebmp/compositeiterator.hxx
@@ -89,26 +89,51 @@ namespace detail
         typedef DifferenceType   difference_type;
         typedef IteratorCategory iterator_category;
 
+        struct Impl
+        {
+            iterator1_type maIter1;
+            iterator2_type maIter2;
+            Impl()
+                : maIter1()
+                , maIter2()
+            {
+            }
+            Impl(const iterator1_type& rIter1, const iterator2_type& rIter2)
+                : maIter1(rIter1)
+                , maIter2(rIter2)
+            {
+            }
+        };
+
     protected:
-        iterator1_type maIter1;
-        iterator2_type maIter2;
+        Impl* pImpl;
 
     private:
         bool equal(CompositeIteratorBase const & rhs) const
         {
-            return (maIter1 == rhs.maIter1) && (maIter2 == rhs.maIter2);
+            return (pImpl->maIter1 == rhs.pImpl->maIter1) && (pImpl->maIter2 == rhs.pImpl->maIter2);
         }
 
     public:
-        CompositeIteratorBase() :
-            maIter1(),
-            maIter2()
-        {}
+        CompositeIteratorBase()
+        {
+            pImpl = new Impl();
+        }
 
-        CompositeIteratorBase( const iterator1_type& rIter1, const iterator2_type& rIter2 ) :
-            maIter1( rIter1 ),
-            maIter2( rIter2 )
-        {}
+        CompositeIteratorBase(const iterator1_type& rIter1, const iterator2_type& rIter2)
+        {
+            pImpl = new Impl(rIter1, rIter2);
+        }
+
+        CompositeIteratorBase(const CompositeIteratorBase& rOther)
+        {
+            pImpl = new Impl(rOther.pImpl->maIter1, rOther.pImpl->maIter2);
+        }
+
+        ~CompositeIteratorBase()
+        {
+            delete pImpl;
+        }
 
         bool operator==(Derived const & rhs) const
         {
@@ -122,21 +147,21 @@ namespace detail
 
         difference_type operator-(Derived const & rhs) const
         {
-            OSL_ASSERT( maIter1 - rhs.maIter1 == maIter2 - rhs.maIter2 );
-            return maIter1 - rhs.maIter1;
+            OSL_ASSERT(pImpl->maIter1 - rhs.pImpl->maIter1 == pImpl->maIter2 - rhs.pImpl->maIter2);
+            return pImpl->maIter1 - rhs.pImpl->maIter1;
         }
 
         Derived & operator+=(difference_type const & s)
         {
-            maIter1 += s;
-            maIter2 += s;
+            pImpl->maIter1 += s;
+            pImpl->maIter2 += s;
             return static_cast<Derived&>(*this);
         }
 
         Derived & operator-=(difference_type const & s)
         {
-            maIter1 -= s;
-            maIter2 -= s;
+            pImpl->maIter1 -= s;
+            pImpl->maIter2 -= s;
             return static_cast<Derived&>(*this);
         }
 
@@ -156,63 +181,70 @@ namespace detail
 
         Derived& operator++()
         {
-            ++maIter1;
-            ++maIter2;
+            ++pImpl->maIter1;
+            ++pImpl->maIter2;
             return static_cast<Derived&>(*this);
         }
 
         Derived& operator--()
         {
-            --maIter1;
-            --maIter2;
+            --pImpl->maIter1;
+            --pImpl->maIter2;
             return static_cast<Derived&>(*this);
         }
 
         Derived operator++(int)
         {
             Derived ret(static_cast<Derived const&>(*this));
-            ++maIter1;
-            ++maIter2;
+            ++pImpl->maIter1;
+            ++pImpl->maIter2;
             return ret;
         }
 
         Derived operator--(int)
         {
             Derived ret(static_cast<Derived const&>(*this));
-            --maIter1;
-            --maIter2;
+            --pImpl->maIter1;
+            --pImpl->maIter2;
             return ret;
         }
 
         value_type get() const
         {
-            return value_type(maIter1.get(),
-                              maIter2.get());
+            return value_type(pImpl->maIter1.get(),
+                              pImpl->maIter2.get());
         }
 
         value_type get(difference_type const & d) const
         {
-            return value_type(maIter1.get(d),
-                              maIter2.get(d));
+            return value_type(pImpl->maIter1.get(d),
+                              pImpl->maIter2.get(d));
         }
 
         void set( value_type v ) const
         {
-            maIter1.set(v);
-            maIter2.set(v);
+            pImpl->maIter1.set(v);
+            pImpl->maIter2.set(v);
         }
 
         void set( value_type v, difference_type const & d ) const
         {
-            maIter1.set(v,d);
-            maIter2.set(v,d);
+            pImpl->maIter1.set(v,d);
+            pImpl->maIter2.set(v,d);
         }
 
-        const iterator1_type& first() const { return maIter1; }
-        iterator1_type& first() { return maIter1; }
+        CompositeIteratorBase& operator=(const CompositeIteratorBase& rNew)
+        {
+            this->pImpl->maIter1 = rNew.pImpl->maIter1;
+            this->pImpl->maIter2 = rNew.pImpl->maIter2;
+            return *this;
+        }
 
-        const iterator2_type& second() const { return maIter2; }
-        iterator2_type& second() { return maIter2; }
+        const iterator1_type& first() const { return pImpl->maIter1; }
+        iterator1_type& first() { return pImpl->maIter1; }
+
+        const iterator2_type& second() const { return pImpl->maIter2; }
+        iterator2_type& second() { return pImpl->maIter2; }
     };
 }
 
@@ -314,43 +346,45 @@ public:
 
     CompositeIterator2D() :
         base_type(),
-        x(this->maIter1.x,this->maIter2.x),
-        y(this->maIter1.y,this->maIter2.y)
+        x(this->pImpl->maIter1.x,this->pImpl->maIter2.x),
+        y(this->pImpl->maIter1.y,this->pImpl->maIter2.y)
     {}
 
     CompositeIterator2D( const Iterator1& rIter1, const Iterator2& rIter2 ) :
         base_type( rIter1, rIter2 ),
-        x(this->maIter1.x,this->maIter2.x),
-        y(this->maIter1.y,this->maIter2.y)
+        x(this->pImpl->maIter1.x,this->pImpl->maIter2.x),
+        y(this->pImpl->maIter1.y,this->pImpl->maIter2.y)
     {}
 
     CompositeIterator2D( const CompositeIterator2D& rOld ) :
         base_type(rOld),
-        x(this->maIter1.x,this->maIter2.x),
-        y(this->maIter1.y,this->maIter2.y)
+        x(this->pImpl->maIter1.x,this->pImpl->maIter2.x),
+        y(this->pImpl->maIter1.y,this->pImpl->maIter2.y)
     {}
 
     CompositeIterator2D& operator=( const CompositeIterator2D& rNew )
     {
-        this->maIter1 = rNew.maIter1;
-        this->maIter2 = rNew.maIter2;
+        this->pImpl->maIter1 = rNew.pImpl->maIter1;
+        this->pImpl->maIter2 = rNew.pImpl->maIter2;
+
+        x = MoveX(this->pImpl->maIter1.x,
+                  this->pImpl->maIter2.x);
+        y = MoveY(this->pImpl->maIter1.y,
+                  this->pImpl->maIter2.y);
 
-        x = MoveX(this->maIter1.x,
-                  this->maIter2.x);
-        y = MoveY(this->maIter1.y,
-                  this->maIter2.y);
+        return *this;
     }
 
     row_iterator rowIterator() const
     {
-        return row_iterator(this->maIter1.rowIterator(),
-                            this->maIter2.rowIterator());
+        return row_iterator(this->pImpl->maIter1.rowIterator(),
+                            this->pImpl->maIter2.rowIterator());
     }
 
     column_iterator columnIterator() const
     {
-        return column_iterator(this->maIter1.columnIterator(),
-                               this->maIter2.columnIterator());
+        return column_iterator(this->pImpl->maIter1.columnIterator(),
+                               this->pImpl->maIter2.columnIterator());
     }
 };
 
commit 319524767ea1f399be6a9491eb053a74abf3dd1e
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Wed Nov 19 10:23:18 2014 +0000

    coverity#1242675 Untrusted value as argument
    
    Change-Id: I1d8f32095f297919dc3ccab51093295f8c31707d

diff --git a/package/source/zipapi/ZipFile.cxx b/package/source/zipapi/ZipFile.cxx
index cd4ed2e..bb178f5 100644
--- a/package/source/zipapi/ZipFile.cxx
+++ b/package/source/zipapi/ZipFile.cxx
@@ -660,11 +660,16 @@ bool ZipFile::readLOC( ZipEntry &rEntry )
 
     try
     {
+        sal_Int16 nPathLenToRead = nPathLen;
+        const sal_Int64 nBytesAvailable = aGrabber.getLength() - aGrabber.getPosition();
+        if (nPathLenToRead > nBytesAvailable)
+            nPathLenToRead = nBytesAvailable;
+
         // read always in UTF8, some tools seem not to set UTF8 bit
-        uno::Sequence < sal_Int8 > aNameBuffer( nPathLen );
-        sal_Int32 nRead = aGrabber.readBytes( aNameBuffer, nPathLen );
-        if ( nRead < aNameBuffer.getLength() )
-                aNameBuffer.realloc( nRead );
+        uno::Sequence<sal_Int8> aNameBuffer(nPathLenToRead);
+        sal_Int32 nRead = aGrabber.readBytes(aNameBuffer, nPathLenToRead);
+        if (nRead < aNameBuffer.getLength())
+            aNameBuffer.realloc(nRead);
 
         OUString sLOCPath = OUString::intern( (sal_Char *) aNameBuffer.getArray(),
                                                           aNameBuffer.getLength(),
commit c3aec587d8fdbdada5a9e26595e7b29e4e6a2920
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Wed Nov 19 10:02:39 2014 +0000

    coverity#735402 Logically dead code
    
    Change-Id: Iafc67b4ccd30afdc2dc1c4fb058f9933fa90185a

diff --git a/cui/source/dialogs/SpellDialog.cxx b/cui/source/dialogs/SpellDialog.cxx
index cb064d6..96b59dc 100644
--- a/cui/source/dialogs/SpellDialog.cxx
+++ b/cui/source/dialogs/SpellDialog.cxx
@@ -1419,7 +1419,7 @@ bool SentenceEditWindow_Impl::PreNotify( NotifyEvent& rNEvt )
                 break;
 //    4 - on field UE and on error CO
                 case FULL       :
-                    nAction = bHasField ? ACTION_UNDOEDIT : ACTION_CONTINUE;
+                    nAction = ACTION_UNDOEDIT;
                 break;
 //    5 - on field FS and on error CO
                 case INSIDE_YES :
commit c7d03c4e9c0bde6cb838b9812e1c0d19191a7e44
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Wed Nov 19 09:58:05 2014 +0000

    coverity#1254674 Unchecked dynamic_cast
    
    Change-Id: Iae62353e42947324beca8d036fcd1c20346d0d29

diff --git a/vcl/inc/openglgdiimpl.hxx b/vcl/inc/openglgdiimpl.hxx
index 29bc7a2..5e4af51 100644
--- a/vcl/inc/openglgdiimpl.hxx
+++ b/vcl/inc/openglgdiimpl.hxx
@@ -230,7 +230,7 @@ public:
 
     // CopyBits and DrawBitmap --> RasterOp and ClipRegion
     // CopyBits() --> pSrcGraphics == NULL, then CopyBits on same Graphics
-    void DoCopyBits( const SalTwoRect& rPosAry, OpenGLSalGraphicsImpl *pSrcImpl );
+    void DoCopyBits(const SalTwoRect& rPosAry, OpenGLSalGraphicsImpl &rSrcImpl);
 
     virtual void drawBitmap( const SalTwoRect& rPosAry, const SalBitmap& rSalBitmap ) SAL_OVERRIDE;
 
diff --git a/vcl/opengl/gdiimpl.cxx b/vcl/opengl/gdiimpl.cxx
index 805c693..3929d63 100644
--- a/vcl/opengl/gdiimpl.cxx
+++ b/vcl/opengl/gdiimpl.cxx
@@ -1301,13 +1301,11 @@ void OpenGLSalGraphicsImpl::copyArea(
 
 // CopyBits and DrawBitmap --> RasterOp and ClipRegion
 // CopyBits() --> pSrcGraphics == NULL, then CopyBits on same Graphics
-void OpenGLSalGraphicsImpl::DoCopyBits( const SalTwoRect& rPosAry, OpenGLSalGraphicsImpl* pSrcImpl )
+void OpenGLSalGraphicsImpl::DoCopyBits( const SalTwoRect& rPosAry, OpenGLSalGraphicsImpl& rImpl )
 {
-    OpenGLSalGraphicsImpl *pImpl = pSrcImpl;
-
     SAL_INFO( "vcl.opengl", "::copyBits" );
 
-    if( pImpl == this &&
+    if( &rImpl == this &&
         (rPosAry.mnSrcWidth == rPosAry.mnDestWidth) &&
         (rPosAry.mnSrcHeight == rPosAry.mnDestHeight))
     {
@@ -1321,10 +1319,10 @@ void OpenGLSalGraphicsImpl::DoCopyBits( const SalTwoRect& rPosAry, OpenGLSalGrap
         return;
     }
 
-    if( pImpl->mbOffscreen )
+    if( rImpl.mbOffscreen )
     {
         PreDraw();
-        DrawTexture( pImpl->maOffscreenTex, rPosAry );
+        DrawTexture( rImpl.maOffscreenTex, rPosAry );
         PostDraw();
         return;
     }
diff --git a/vcl/opengl/win/gdiimpl.cxx b/vcl/opengl/win/gdiimpl.cxx
index 67a192e..18e0da8 100644
--- a/vcl/opengl/win/gdiimpl.cxx
+++ b/vcl/opengl/win/gdiimpl.cxx
@@ -20,8 +20,8 @@ WinOpenGLSalGraphicsImpl::WinOpenGLSalGraphicsImpl(WinSalGraphics& rGraphics):
 
 void WinOpenGLSalGraphicsImpl::copyBits( const SalTwoRect& rPosAry, SalGraphics* pSrcGraphics )
 {
-    OpenGLSalGraphicsImpl *pImpl = pSrcGraphics ? dynamic_cast< OpenGLSalGraphicsImpl* >(pSrcGraphics->GetImpl()) : static_cast< OpenGLSalGraphicsImpl *>(mrParent.GetImpl());
-    OpenGLSalGraphicsImpl::DoCopyBits( rPosAry, pImpl );
+    OpenGLSalGraphicsImpl *pImpl = pSrcGraphics ? static_cast< OpenGLSalGraphicsImpl* >(pSrcGraphics->GetImpl()) : static_cast< OpenGLSalGraphicsImpl *>(mrParent.GetImpl());
+    OpenGLSalGraphicsImpl::DoCopyBits( rPosAry, *pImpl );
 }
 
 GLfloat WinOpenGLSalGraphicsImpl::GetWidth() const
diff --git a/vcl/opengl/x11/gdiimpl.cxx b/vcl/opengl/x11/gdiimpl.cxx
index dcefcc2..b3cee49 100644
--- a/vcl/opengl/x11/gdiimpl.cxx
+++ b/vcl/opengl/x11/gdiimpl.cxx
@@ -83,8 +83,8 @@ void X11OpenGLSalGraphicsImpl::Init()
 
 void X11OpenGLSalGraphicsImpl::copyBits( const SalTwoRect& rPosAry, SalGraphics* pSrcGraphics )
 {
-    OpenGLSalGraphicsImpl *pImpl = pSrcGraphics ? dynamic_cast< OpenGLSalGraphicsImpl* >(pSrcGraphics->GetImpl()) : static_cast< OpenGLSalGraphicsImpl *>(mrParent.GetImpl());
-    OpenGLSalGraphicsImpl::DoCopyBits( rPosAry, pImpl );
+    OpenGLSalGraphicsImpl *pImpl = pSrcGraphics ? static_cast< OpenGLSalGraphicsImpl* >(pSrcGraphics->GetImpl()) : static_cast< OpenGLSalGraphicsImpl *>(mrParent.GetImpl());
+    OpenGLSalGraphicsImpl::DoCopyBits( rPosAry, *pImpl );
 }
 
 bool X11OpenGLSalGraphicsImpl::FillPixmapFromScreen( X11Pixmap* pPixmap, int nX, int nY )
commit 71f1553cee2c24a4e5b863750e5cfb795b54ecc2
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Wed Nov 19 09:50:40 2014 +0000

    Related: coverity#1242658 Untrusted loop bound
    
    Change-Id: Ic785b333c1ad31036da20483dc7310fcc5338d80

diff --git a/filter/source/graphicfilter/ipict/ipict.cxx b/filter/source/graphicfilter/ipict/ipict.cxx
index dec3fc6..852e69b 100644
--- a/filter/source/graphicfilter/ipict/ipict.cxx
+++ b/filter/source/graphicfilter/ipict/ipict.cxx
@@ -990,7 +990,11 @@ sal_uLong PictReader::ReadPixMapEtc( Bitmap &rBitmap, bool bBaseAddr, bool bColo
         if ( nRowBytes < 8 || nPackType == 1 )
         {
             const size_t nMaxPixels = pPict->remainingSize() / 4;
-            if (static_cast<size_t>(nHeight) * nWidth > nMaxPixels)
+            const size_t nMaxRows = nMaxPixels / nWidth;
+            if (nHeight > nMaxRows)
+                BITMAPERROR;
+            const size_t nMaxCols = nMaxPixels / nHeight;
+            if (nWidth > nMaxCols)
                 BITMAPERROR;
 
             for ( ny = 0; ny < nHeight; ny++ )
@@ -1006,7 +1010,11 @@ sal_uLong PictReader::ReadPixMapEtc( Bitmap &rBitmap, bool bBaseAddr, bool bColo
         else if ( nPackType == 2 )
         {
             const size_t nMaxPixels = pPict->remainingSize() / 3;
-            if (static_cast<size_t>(nHeight) * nWidth > nMaxPixels)
+            const size_t nMaxRows = nMaxPixels / nWidth;
+            if (nHeight > nMaxRows)
+                BITMAPERROR;
+            const size_t nMaxCols = nMaxPixels / nHeight;
+            if (nWidth > nMaxCols)
                 BITMAPERROR;
 
             for ( ny = 0; ny < nHeight; ny++ )
commit 885494f4b5e45bde3ecb029796930bfa6741512b
Author: Caolán McNamara <caolanm at redhat.com>
Date:   Wed Nov 19 09:34:13 2014 +0000

    coverity#1242627 Use of untrusted scalar value
    
    Change-Id: I81b034859e80507740092b25a7352c19933e8449

diff --git a/vcl/source/control/field.cxx b/vcl/source/control/field.cxx
index 53f0ac9..73d8e9a 100644
--- a/vcl/source/control/field.cxx
+++ b/vcl/source/control/field.cxx
@@ -1367,7 +1367,12 @@ void MetricFormatter::ImplLoadRes( const ResId& rResId )
         sal_uLong       nMask = pMgr->ReadLong();
 
         if ( METRICFORMATTER_UNIT & nMask )
-            meUnit = (FieldUnit)pMgr->ReadLong();
+        {
+            sal_uLong nUnit = pMgr->ReadLong();
+            assert(nUnit <= FUNIT_MILLISECOND && "out of FieldUnit bounds");
+            if (nUnit <= FUNIT_MILLISECOND)
+                meUnit = (FieldUnit)nUnit;
+        }
 
         if ( METRICFORMATTER_CUSTOMUNITTEXT & nMask )
             maCustomUnitText = pMgr->ReadString();


More information about the Libreoffice-commits mailing list