[Libreoffice-commits] core.git: 10 commits - compilerplugins/clang cppcanvas/source cppu/source libreofficekit/qa lotuswordpro/source oox/source package/inc package/source sax/source sc/source

Stephan Bergmann sbergman at redhat.com
Thu Feb 23 07:49:57 UTC 2017


 compilerplugins/clang/subtlezeroinit.cxx            |   59 ++++++++++++++++++++
 cppcanvas/source/mtfrenderer/emfplus.cxx            |   12 +++-
 cppu/source/threadpool/current.cxx                  |    2 
 cppu/source/threadpool/threadident.cxx              |    1 
 libreofficekit/qa/gtktiledviewer/gtktiledviewer.cxx |    4 +
 lotuswordpro/source/filter/lwpfrib.cxx              |    3 -
 lotuswordpro/source/filter/lwpidxmgr.cxx            |   12 ++--
 lotuswordpro/source/filter/lwpidxmgr.hxx            |    2 
 oox/source/crypto/DocumentDecryption.cxx            |    7 ++
 package/inc/ZipPackageFolder.hxx                    |    1 
 package/source/zippackage/ZipPackageFolder.cxx      |   18 ------
 package/source/zippackage/ZipPackageStream.cxx      |   11 +--
 sax/source/fastparser/fastparser.cxx                |    3 -
 sc/source/core/data/colorscale.cxx                  |    5 +
 14 files changed, 101 insertions(+), 39 deletions(-)

New commits:
commit 2c952c6a59c508e330386124df15a04217c3301c
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 22:26:42 2017 +0100

    New loplugin:subtlezeroinit
    
    Change-Id: I4530021d78f714d389833bd00ea87430d1d20f52

diff --git a/compilerplugins/clang/subtlezeroinit.cxx b/compilerplugins/clang/subtlezeroinit.cxx
new file mode 100644
index 0000000..c6c3486
--- /dev/null
+++ b/compilerplugins/clang/subtlezeroinit.cxx
@@ -0,0 +1,59 @@
+/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
+/*
+ * This file is part of the LibreOffice project.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+#include "plugin.hxx"
+
+// Find occurrences of 'new T()' where the instance is zero-initialized upfront
+// since C++11.  For one, in many cases this may be unnecessary and unintended,
+// as the code was written before C++11.  For another, the zero-initialization
+// would go away when T gets a user-provided default constructor, for example,
+// so better make any necessary initialization more explicit in the code.
+
+namespace {
+
+class Visitor final:
+    public RecursiveASTVisitor<Visitor>, public loplugin::Plugin
+{
+public:
+    explicit Visitor(InstantiationData const & data): Plugin(data) {}
+
+    bool VisitCXXNewExpr(CXXNewExpr const * expr) {
+        if (ignoreLocation(expr)) {
+            return true;
+        }
+        auto ce = expr->getConstructExpr();
+        if (ce == nullptr) {
+            return true;
+        }
+        if (!ce->requiresZeroInitialization()) {
+            return true;
+        }
+        report(
+            DiagnosticsEngine::Warning,
+            ("if zero-initialization of %0 is intentional here, better make"
+             " that more explicit (e.g., assigning to members, default"
+             " constructor, default member initializers, std::memset)"),
+            expr->getExprLoc())
+            << ce->getType() << expr->getSourceRange();
+        return true;
+    }
+
+private:
+    void run() override {
+        if (compiler.getLangOpts().CPlusPlus) {
+            TraverseDecl(compiler.getASTContext().getTranslationUnitDecl());
+        }
+    }
+};
+
+static loplugin::Plugin::Registration<Visitor> reg("subtlezeroinit");
+
+}
+
+/* vim:set shiftwidth=4 softtabstop=4 expandtab cinoptions=b1,g0,N-s cinkeys+=0=break: */
commit 43f2a7d979eac630309064e5de3af1f22ba168e9
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 22:02:48 2017 +0100

    Better make the zero-initialization more explicit
    
    ...if initializing these members is necessary at all; a bit hard to tell from
    the code.
    
    Change-Id: I3ddb25c35f0d1b44a092bdb2cb0192ce619f947c

diff --git a/cppcanvas/source/mtfrenderer/emfplus.cxx b/cppcanvas/source/mtfrenderer/emfplus.cxx
index bb8f0a8..97f6e01 100644
--- a/cppcanvas/source/mtfrenderer/emfplus.cxx
+++ b/cppcanvas/source/mtfrenderer/emfplus.cxx
@@ -693,7 +693,12 @@ namespace cppcanvas
             case EmfPlusObjectTypeImage:
                 {
                     EMFPImage *image;
-                    aObjects [index] = image = new EMFPImage ();
+                    aObjects [index] = image = new EMFPImage;
+                    image->type = 0;
+                    image->width = 0;
+                    image->height = 0;
+                    image->stride = 0;
+                    image->pixelFormat = 0;
                     image->Read (rObjectStream, dataSize, bUseWholeStream);
 
                     break;
@@ -701,7 +706,10 @@ namespace cppcanvas
             case EmfPlusObjectTypeFont:
                 {
                     EMFPFont *font;
-                    aObjects [index] = font = new EMFPFont ();
+                    aObjects [index] = font = new EMFPFont;
+                    font->emSize = 0;
+                    font->sizeUnit = 0;
+                    font->fontFlags = 0;
                     font->Read (rObjectStream);
 
                     break;
commit 802ac3e437bff5bfbc530c91105f0c380abd62fc
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:56:18 2017 +0100

    Assert IdContainer's bInit in uno_releaseIdFromCurrentThread
    
    ...implying that pLocalThreadId, nRefCountOfCurrentId, pCurrentId are
    initialized, implying that zero-initializing them during construction isn't
    necessary.
    
    Change-Id: I98399203694edde14abc664a82861ba50dfb357c

diff --git a/cppu/source/threadpool/current.cxx b/cppu/source/threadpool/current.cxx
index eafb0bb..1b82a02 100644
--- a/cppu/source/threadpool/current.cxx
+++ b/cppu/source/threadpool/current.cxx
@@ -163,7 +163,7 @@ IdContainer * getIdContainer()
     IdContainer * pId = static_cast< IdContainer * >( ::osl_getThreadKeyData( aKey ) );
     if (! pId)
     {
-        pId = new IdContainer();
+        pId = new IdContainer;
         pId->pCurrentContext = nullptr;
         pId->pCurrentContextEnv = nullptr;
         pId->bInit = false;
diff --git a/cppu/source/threadpool/threadident.cxx b/cppu/source/threadpool/threadident.cxx
index a9b1dce..24037f6 100644
--- a/cppu/source/threadpool/threadident.cxx
+++ b/cppu/source/threadpool/threadident.cxx
@@ -85,6 +85,7 @@ extern "C" void SAL_CALL uno_releaseIdFromCurrentThread()
 {
     IdContainer *p = getIdContainer();
     OSL_ASSERT( p );
+    OSL_ASSERT( p->bInit );
     OSL_ASSERT( p->nRefCountOfCurrentId );
 
     p->nRefCountOfCurrentId --;
commit 3534eef59d04ca22323330245fe91c758072f758
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:45:19 2017 +0100

    Better make the zero-initialization more explicit
    
    ...if initializing these members is necessary at all; a bit hard to tell from
    the code.
    
    Change-Id: I384fbeedb3a5b0690a80cabdd13ca13d07e8d627

diff --git a/libreofficekit/qa/gtktiledviewer/gtktiledviewer.cxx b/libreofficekit/qa/gtktiledviewer/gtktiledviewer.cxx
index 59faff1..0ad1c2e 100644
--- a/libreofficekit/qa/gtktiledviewer/gtktiledviewer.cxx
+++ b/libreofficekit/qa/gtktiledviewer/gtktiledviewer.cxx
@@ -1148,7 +1148,9 @@ static void initWindow(TiledWindow& rWindow)
     {
         if (!rWindow.m_pCommentsSidebar)
         {
-            rWindow.m_pCommentsSidebar.reset(new CommentsSidebar());
+            rWindow.m_pCommentsSidebar.reset(new CommentsSidebar);
+            rWindow.m_pCommentsSidebar->m_pCommentsVBox = nullptr;
+            rWindow.m_pCommentsSidebar->m_pScrolledWindow = nullptr;
             rWindow.m_pCommentsSidebar->m_pMainVBox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 0);
             gtk_container_add(GTK_CONTAINER(rWindow.m_pMainHBox), rWindow.m_pCommentsSidebar->m_pMainVBox);
 
commit 2122952e740f345be1d905d77fcf0b7ed4278c07
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:28:42 2017 +0100

    Better make the zero-initialization more explicit
    
    ...if initializing offset is necessary at all; a bit hard to tell from the code.
    
    Change-Id: Ic1d7c97f174e3ed04b03f4004ca858029e9a258e

diff --git a/lotuswordpro/source/filter/lwpidxmgr.cxx b/lotuswordpro/source/filter/lwpidxmgr.cxx
index 3b6b3a7..ff7fc21 100644
--- a/lotuswordpro/source/filter/lwpidxmgr.cxx
+++ b/lotuswordpro/source/filter/lwpidxmgr.cxx
@@ -159,7 +159,7 @@ void LwpIndexManager::ReadRootData(LwpObjectStream* pObjStrm)
     if (KeyCount)
     {
         //read object keys
-        LwpKey* akey = new LwpKey();
+        LwpKey* akey = new LwpKey;
         akey->id.Read(pObjStrm);
         m_RootObjs.push_back(akey);
 
@@ -167,7 +167,7 @@ void LwpIndexManager::ReadRootData(LwpObjectStream* pObjStrm)
 
         for (k = 1; k < KeyCount; k++)
         {
-            akey = new LwpKey();
+            akey = new LwpKey;
             akey->id.ReadCompressed(pObjStrm, m_RootObjs[k-1]->id);
             m_RootObjs.push_back(akey);
         }
@@ -197,7 +197,7 @@ void LwpIndexManager::ReadObjIndexData(LwpObjectStream* pObjStrm)
 
     if(KeyCount)
     {
-        LwpKey* akey = new LwpKey();
+        LwpKey* akey = new LwpKey;
         akey->id.Read(pObjStrm);
         vObjIndexs.push_back(akey);
 
@@ -205,7 +205,7 @@ void LwpIndexManager::ReadObjIndexData(LwpObjectStream* pObjStrm)
 
         for (k = 1; k < KeyCount; k++)
         {
-            akey = new LwpKey();
+            akey = new LwpKey;
             akey->id.ReadCompressed(pObjStrm, vObjIndexs[k-1]->id);
             vObjIndexs.push_back(akey);
         }
@@ -281,14 +281,14 @@ void LwpIndexManager::ReadLeafData( LwpObjectStream *pObjStrm )
 
     if(KeyCount)
     {
-        LwpKey* akey = new LwpKey();
+        LwpKey* akey = new LwpKey;
         //read object keys: id & offset
         akey->id.Read(pObjStrm);
         m_ObjectKeys.push_back(akey);
 
         for (sal_uInt16 k = 1; k < KeyCount; k++)
         {
-            akey = new LwpKey();
+            akey = new LwpKey;
             akey->id.ReadCompressed(pObjStrm, m_ObjectKeys.at(m_nKeyCount+k-1)->id);
             m_ObjectKeys.push_back(akey);
         }
diff --git a/lotuswordpro/source/filter/lwpidxmgr.hxx b/lotuswordpro/source/filter/lwpidxmgr.hxx
index d1e9ff6..c07bb98 100644
--- a/lotuswordpro/source/filter/lwpidxmgr.hxx
+++ b/lotuswordpro/source/filter/lwpidxmgr.hxx
@@ -71,7 +71,7 @@
 struct LwpKey
 {
     LwpObjectID id;
-    sal_uInt32 offset;
+    sal_uInt32 offset = 0;
 };
 /**
  * @brief   LwpIndexManager, to read all index records and maintain the index information
commit bf8627cbb7c40925eb29af004b9bd3d58322bcfa
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:25:58 2017 +0100

    Better make the zero-initialization more explicit
    
    ...if initializing RevisionType is necessary at all; a bit hard to tell from
    the code.
    
    Change-Id: Ic472c8cceb818b7e18d3de348f392f8641d85255

diff --git a/lotuswordpro/source/filter/lwpfrib.cxx b/lotuswordpro/source/filter/lwpfrib.cxx
index 3b41993..a056216 100644
--- a/lotuswordpro/source/filter/lwpfrib.cxx
+++ b/lotuswordpro/source/filter/lwpfrib.cxx
@@ -107,9 +107,10 @@ LwpFrib* LwpFrib::CreateFrib(LwpPara* pPara, LwpObjectStream* pObjStrm, sal_uInt
     ModifierInfo* pModInfo = nullptr;
     if(fribtag & FRIB_TAG_MODIFIER)
     {
-        pModInfo  = new ModifierInfo();
+        pModInfo  = new ModifierInfo;
         pModInfo->CodePage = 0;
         pModInfo->FontID = 0;
+        pModInfo->RevisionType = 0;
         pModInfo->RevisionFlag = false;
         pModInfo->HasCharStyle = false;
         pModInfo->HasLangOverride = false;
commit 27728d21e479007a7534e2759e464ebfe10a4e70
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:22:34 2017 +0100

    Better make the zero-initialization more explicit
    
    ...if initializing these members is necessary at all; a bit hard to tell from
    the code.
    
    Change-Id: If15fb11b601ba3b8b5d3fdbe5c3dac01b095fb05

diff --git a/oox/source/crypto/DocumentDecryption.cxx b/oox/source/crypto/DocumentDecryption.cxx
index f93f903..0209f3d 100644
--- a/oox/source/crypto/DocumentDecryption.cxx
+++ b/oox/source/crypto/DocumentDecryption.cxx
@@ -191,9 +191,14 @@ bool DocumentDecryption::generateEncryptionKey(const OUString& rPassword)
 
 bool DocumentDecryption::readAgileEncryptionInfo(Reference< XInputStream >& xInputStream)
 {
-    AgileEngine* engine = new AgileEngine();
+    AgileEngine* engine = new AgileEngine;
     mEngine.reset(engine);
     AgileEncryptionInfo& info = engine->getInfo();
+    info.spinCount = 0;
+    info.saltSize = 0;
+    info.keyBits = 0;
+    info.hashSize = 0;
+    info.blockSize = 0;
 
     Reference<XFastDocumentHandler> xFastDocumentHandler( new AgileDocumentHandler(info) );
     Reference<XFastTokenHandler>    xFastTokenHandler   ( new AgileTokenHandler );
commit 82ea4fdc37c475c4a39842341b6f236c689ff755
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:16:56 2017 +0100

    Use copy ctor instead of ZipPackageFolder::copyZipEntry
    
    Change-Id: Ief103f2f4c66a2086f73c4d2ff332e769e6fc33b

diff --git a/package/inc/ZipPackageFolder.hxx b/package/inc/ZipPackageFolder.hxx
index fb6a072..e638cc6 100644
--- a/package/inc/ZipPackageFolder.hxx
+++ b/package/inc/ZipPackageFolder.hxx
@@ -64,7 +64,6 @@ public:
 
     ZipContentInfo& doGetByName( const OUString& aName );
 
-    static void copyZipEntry( ZipEntry &rDest, const ZipEntry &rSource);
     static css::uno::Sequence < sal_Int8 > static_getImplementationId();
 
     void setPackageFormat_Impl( sal_Int32 nFormat ) { m_nFormat = nFormat; }
diff --git a/package/source/zippackage/ZipPackageFolder.cxx b/package/source/zippackage/ZipPackageFolder.cxx
index 8bf0a88..e7f0cca 100644
--- a/package/source/zippackage/ZipPackageFolder.cxx
+++ b/package/source/zippackage/ZipPackageFolder.cxx
@@ -159,21 +159,6 @@ void ZipPackageFolder::setChildStreamsTypeByExtension( const beans::StringPair&
     }
 }
 
-void ZipPackageFolder::copyZipEntry( ZipEntry &rDest, const ZipEntry &rSource)
-{
-      rDest.nVersion            = rSource.nVersion;
-    rDest.nFlag             = rSource.nFlag;
-    rDest.nMethod           = rSource.nMethod;
-    rDest.nTime             = rSource.nTime;
-    rDest.nCrc              = rSource.nCrc;
-    rDest.nCompressedSize   = rSource.nCompressedSize;
-    rDest.nSize             = rSource.nSize;
-    rDest.nOffset           = rSource.nOffset;
-    rDest.sPath             = rSource.sPath;
-    rDest.nPathLen          = rSource.nPathLen;
-    rDest.nExtraLen         = rSource.nExtraLen;
-}
-
 css::uno::Sequence < sal_Int8 > ZipPackageFolder::static_getImplementationId()
 {
     return lcl_CachedImplId::get().getImplementationId();
@@ -320,8 +305,7 @@ void ZipPackageFolder::saveContents(
     if ( maContents.empty() && !rPath.isEmpty() && m_nFormat != embed::StorageFormats::OFOPXML )
     {
         // it is an empty subfolder, use workaround to store it
-        ZipEntry* pTempEntry = new ZipEntry();
-        ZipPackageFolder::copyZipEntry ( *pTempEntry, aEntry );
+        ZipEntry* pTempEntry = new ZipEntry(aEntry);
         pTempEntry->nPathLen = (sal_Int16)( OUStringToOString( rPath, RTL_TEXTENCODING_UTF8 ).getLength() );
         pTempEntry->nExtraLen = -1;
         pTempEntry->sPath = rPath;
diff --git a/package/source/zippackage/ZipPackageStream.cxx b/package/source/zippackage/ZipPackageStream.cxx
index d684493..7f54260 100644
--- a/package/source/zippackage/ZipPackageStream.cxx
+++ b/package/source/zippackage/ZipPackageStream.cxx
@@ -520,15 +520,14 @@ bool ZipPackageStream::saveChild(
 
     uno::Sequence < beans::PropertyValue > aPropSet (PKG_SIZE_NOENCR_MNFST);
 
+    // In case the entry we are reading is also the entry we are writing, we will
+    // store the ZipEntry data in pTempEntry
+
     // if pTempEntry is necessary, it will be released and passed to the ZipOutputStream
     // and be deleted in the ZipOutputStream destructor
-    std::unique_ptr < ZipEntry > pAutoTempEntry ( new ZipEntry );
+    std::unique_ptr < ZipEntry > pAutoTempEntry ( new ZipEntry(aEntry) );
     ZipEntry* pTempEntry = pAutoTempEntry.get();
 
-    // In case the entry we are reading is also the entry we are writing, we will
-    // store the ZipEntry data in pTempEntry
-
-    ZipPackageFolder::copyZipEntry ( *pTempEntry, aEntry );
     pTempEntry->sPath = rPath;
     pTempEntry->nPathLen = (sal_Int16)( OUStringToOString( pTempEntry->sPath, RTL_TEXTENCODING_UTF8 ).getLength() );
 
@@ -904,7 +903,7 @@ void ZipPackageStream::successfullyWritten( ZipEntry *pEntry )
     }
 
     // Then copy it back afterwards...
-    ZipPackageFolder::copyZipEntry( aEntry, *pEntry );
+    aEntry = *pEntry;
 
     // TODO/LATER: get rid of this hack ( the encrypted stream size property is changed during saving )
     if ( m_bIsEncrypted )
commit 3d441992df13d62109dc8385999ccc5ebe4bf338
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:12:54 2017 +0100

    Better make the zero-initialization more explicit
    
    ...if initializing mbIsAttributesEmpty is necessary at all; a bit hard to tell
    from the code.
    
    Change-Id: Ia19014b4bf6bb2c41d11ee0afb43fe6bf7da1fcb

diff --git a/sax/source/fastparser/fastparser.cxx b/sax/source/fastparser/fastparser.cxx
index 1b0ad53..78378dc 100644
--- a/sax/source/fastparser/fastparser.cxx
+++ b/sax/source/fastparser/fastparser.cxx
@@ -544,8 +544,9 @@ EventList* Entity::getEventList()
         }
         if (!mpProducedEvents)
         {
-            mpProducedEvents = new EventList();
+            mpProducedEvents = new EventList;
             mpProducedEvents->maEvents.resize(mnEventListSize);
+            mpProducedEvents->mbIsAttributesEmpty = false;
             mnProducedEventsSize = 0;
         }
     }
commit 189bfa9c9946fde2f8bc791aad501f72ca45db5b
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Wed Feb 22 21:05:27 2017 +0100

    All ScAxisPosition values that meAxisPosition can take on are covered
    
    ...so all of pInfo's members are initialized, so no need to first zero-
    initialize it
    
    Change-Id: I9cc7a0cc038aec59dca4121d16164843483737fc

diff --git a/sc/source/core/data/colorscale.cxx b/sc/source/core/data/colorscale.cxx
index dbce784..d29e25f 100644
--- a/sc/source/core/data/colorscale.cxx
+++ b/sc/source/core/data/colorscale.cxx
@@ -20,6 +20,7 @@
 #include <o3tl/make_unique.hxx>
 
 #include <algorithm>
+#include <cassert>
 
 ScFormulaListener::ScFormulaListener(ScFormulaCell* pCell):
     mbDirty(false),
@@ -770,7 +771,7 @@ ScDataBarInfo* ScDataBarFormat::GetDataBarInfo(const ScAddress& rAddr) const
 
     double nValue = rCell.getValue();
 
-    ScDataBarInfo* pInfo = new ScDataBarInfo();
+    ScDataBarInfo* pInfo = new ScDataBarInfo;
     if(mpFormatData->meAxisPosition == databar::NONE)
     {
         if(nValue <= nMin)
@@ -849,6 +850,8 @@ ScDataBarInfo* ScDataBarFormat::GetDataBarInfo(const ScAddress& rAddr) const
                 pInfo->mnLength = nMaxLength * (std::max(nValue, nMin)/nAbsMax);
         }
     }
+    else
+        assert(false);
 
     // set color
     if(mpFormatData->mbNeg && nValue < 0)


More information about the Libreoffice-commits mailing list