[PATCH] Bug # 51154: cppunit warning cleaning

Hi,

I would like you to review the following patch:

    https://gerrit.libreoffice.org/5424

To pull it, you can do:

    git pull ssh://gerrit.libreoffice.org:29418/cppunit refs/changes/24/5424/1

Bug # 51154: cppunit warning cleaning

This patch allows to compile the code with gcc's -Weffc++

It consists mostly of making copy constructors and assignment operators
explicit and private, and of initializing all members in initializer lists.

Change-Id: I6f1cae812c58e3791c2386a1288501cf2f559610
---
M include/cppunit/Message.h
M include/cppunit/Protector.h
M include/cppunit/SynchronizedObject.h
M include/cppunit/XmlOutputter.h
M include/cppunit/plugin/PlugInManager.h
M include/cppunit/ui/text/TextTestRunner.h
M src/cppunit/DynamicLibraryManagerException.cpp
M src/cppunit/Exception.cpp
M src/cppunit/Message.cpp
M src/cppunit/PlugInManager.cpp
M src/cppunit/ProtectorChain.cpp
M src/cppunit/ProtectorChain.h
M src/cppunit/ProtectorContext.h
M src/cppunit/SourceLine.cpp
M src/cppunit/TestCase.cpp
M src/cppunit/TestFactoryRegistry.cpp
M src/cppunit/TestNamer.cpp
M src/cppunit/TestPath.cpp
M src/cppunit/TestResult.cpp
M src/cppunit/TestResultCollector.cpp
M src/cppunit/TestSuite.cpp
M src/cppunit/TestSuiteBuilderContext.cpp
M src/cppunit/XmlDocument.cpp
M src/cppunit/XmlElement.cpp
M src/cppunit/XmlOutputter.cpp
25 files changed, 86 insertions(+), 24 deletions(-)



diff --git a/include/cppunit/Message.h b/include/cppunit/Message.h
index 7c462d5..5b5e4ec 100644
--- a/include/cppunit/Message.h
+++ b/include/cppunit/Message.h
@@ -38,7 +38,7 @@
 class CPPUNIT_API Message
 {
 public:
-  Message();
+  Message() {};
 
   // Ensure thread-safe copy by detaching the string.
   Message( const Message &other );
@@ -57,7 +57,7 @@
            const std::string &detail2,
            const std::string &detail3 );
 
-  ~Message();
+  virtual ~Message();
 
   Message &operator =( const Message &other );
 
diff --git a/include/cppunit/Protector.h b/include/cppunit/Protector.h
index d14e75f..c6d2e7c 100644
--- a/include/cppunit/Protector.h
+++ b/include/cppunit/Protector.h
@@ -84,6 +84,8 @@
   ~ProtectorGuard();
 
 private:
+  ProtectorGuard( const ProtectorGuard& ); /* not copyable */
+  ProtectorGuard& operator=( const ProtectorGuard& ); /* not assignable */
   TestResult *m_result;
 };
 
diff --git a/include/cppunit/SynchronizedObject.h b/include/cppunit/SynchronizedObject.h
index 0f7d094..59c3cbb 100644
--- a/include/cppunit/SynchronizedObject.h
+++ b/include/cppunit/SynchronizedObject.h
@@ -50,15 +50,21 @@
 
   public:
     ExclusiveZone( SynchronizationObject *syncObject ) 
-        : m_syncObject( syncObject ) 
+        : m_syncObject( syncObject )
     { 
-      m_syncObject->lock(); 
+      m_syncObject->lock();
     }
 
     ~ExclusiveZone() 
     { 
-      m_syncObject->unlock (); 
+      m_syncObject->unlock ();
     }
+  private:
+    /// Prevents the use of the copy constructor.
+    ExclusiveZone( const ExclusiveZone& );
+
+    /// Prevents the use of the copy operator.
+    ExclusiveZone& operator=( const ExclusiveZone& );
   };
 
   virtual void setSynchronizationObject( SynchronizationObject *syncObject );
diff --git a/include/cppunit/XmlOutputter.h b/include/cppunit/XmlOutputter.h
index 0de9676..da8164a 100644
--- a/include/cppunit/XmlOutputter.h
+++ b/include/cppunit/XmlOutputter.h
@@ -46,7 +46,7 @@
    */
   XmlOutputter( TestResultCollector *result,
                 OStream &stream,
-                std::string encoding = std::string("ISO-8859-1") );
+                const std::string& encoding = std::string("ISO-8859-1") );
 
   /// Destructor.
   virtual ~XmlOutputter();
diff --git a/include/cppunit/plugin/PlugInManager.h b/include/cppunit/plugin/PlugInManager.h
index 6ecedc8..bdc4e9f 100644
--- a/include/cppunit/plugin/PlugInManager.h
+++ b/include/cppunit/plugin/PlugInManager.h
@@ -74,6 +74,8 @@
   void removeXmlOutputterHooks();
 
 protected:
+// disable warnings for PODs
+#pragma GCC diagnostic ignored "-Weffc++"
   /*! \brief (INTERNAL) Information about a specific plug-in.
    */
   struct PlugInInfo
@@ -82,6 +84,7 @@
     DynamicLibraryManager *m_manager;
     CppUnitTestPlugIn *m_interface;
   };
+#pragma GCC diagnostic pop
 
   /*! Unloads the specified plug-in.
    * \param plugIn Information about the plug-in.
diff --git a/include/cppunit/ui/text/TextTestRunner.h b/include/cppunit/ui/text/TextTestRunner.h
index 86da4d4..6250166 100644
--- a/include/cppunit/ui/text/TextTestRunner.h
+++ b/include/cppunit/ui/text/TextTestRunner.h
@@ -86,6 +86,12 @@
   virtual void wait( bool doWait );
   virtual void printResult( bool doPrintResult );
 
+private:
+  // prohibit copying
+  TextTestRunner( const TextTestRunner& );
+  // prohibit copying
+  TextTestRunner& operator=( const TextTestRunner& );
+
   TestResultCollector *m_result;
   TestResult *m_eventManager;
   Outputter *m_outputter;
diff --git a/src/cppunit/DynamicLibraryManagerException.cpp 
b/src/cppunit/DynamicLibraryManagerException.cpp
index 8498652..d5a89d8 100644
--- a/src/cppunit/DynamicLibraryManagerException.cpp
+++ b/src/cppunit/DynamicLibraryManagerException.cpp
@@ -4,13 +4,13 @@
 
 CPPUNIT_NS_BEGIN
 
-
 DynamicLibraryManagerException::DynamicLibraryManagerException( 
                                          const std::string &libraryName,
                                          const std::string &errorDetail,
                                          Cause cause )
-    : std::runtime_error( "" ),
-      m_cause( cause )
+    : std::runtime_error( "" )
+    , m_message()
+    , m_cause( cause )
 {
   if ( cause == loadingFailed )
     m_message = "Failed to load dynamic library: " + libraryName + "\n" + 
@@ -19,7 +19,6 @@
     m_message = "Symbol [" + errorDetail + "] not found in dynamic libary:" + 
                 libraryName;
 }
-
 
 DynamicLibraryManagerException::Cause 
 DynamicLibraryManagerException::getCause() const
diff --git a/src/cppunit/Exception.cpp b/src/cppunit/Exception.cpp
index 3bbe24b..6685480 100644
--- a/src/cppunit/Exception.cpp
+++ b/src/cppunit/Exception.cpp
@@ -19,19 +19,19 @@
 
 Exception::Exception( const Exception &other )
    : std::exception( other )
+   , m_message(other.m_message)
+   , m_sourceLine(other.m_sourceLine)
+   , m_whatMessage(other.m_whatMessage)
 { 
-  m_message = other.m_message; 
-  m_sourceLine = other.m_sourceLine;
 } 
-
 
 Exception::Exception( const Message &message, 
                       const SourceLine &sourceLine )
     : m_message( message )
     , m_sourceLine( sourceLine )
+    , m_whatMessage()
 {
 }
-
 
 #ifdef CPPUNIT_ENABLE_SOURCELINE_DEPRECATED
 Exception::Exception( std::string message, 
diff --git a/src/cppunit/Message.cpp b/src/cppunit/Message.cpp
index 955a381..eed64c6 100644
--- a/src/cppunit/Message.cpp
+++ b/src/cppunit/Message.cpp
@@ -4,19 +4,16 @@
 
 CPPUNIT_NS_BEGIN
 
-
-Message::Message()
-{
-}
-
 Message::Message( const Message &other )
+       : m_shortDescription()
+       , m_details()
 {
    *this = other;
 }
 
-
 Message::Message( const std::string &shortDescription )
     : m_shortDescription( shortDescription )
+       , m_details()
 {
 }
 
@@ -24,6 +21,7 @@
 Message::Message( const std::string &shortDescription,
                   const std::string &detail1 )
     : m_shortDescription( shortDescription )
+       , m_details()
 {
   addDetail( detail1 );
 }
@@ -33,6 +31,7 @@
                   const std::string &detail1,
                   const std::string &detail2 )
     : m_shortDescription( shortDescription )
+       , m_details()
 {
   addDetail( detail1, detail2 );
 }
@@ -43,6 +42,7 @@
                   const std::string &detail2,
                   const std::string &detail3 )
     : m_shortDescription( shortDescription )
+       , m_details()
 {
   addDetail( detail1, detail2, detail3 );
 }
diff --git a/src/cppunit/PlugInManager.cpp b/src/cppunit/PlugInManager.cpp
index 7343094..69dcf1d 100644
--- a/src/cppunit/PlugInManager.cpp
+++ b/src/cppunit/PlugInManager.cpp
@@ -13,6 +13,7 @@
 
 
 PlugInManager::PlugInManager()
+       : m_plugIns()
 {
 }
 
diff --git a/src/cppunit/ProtectorChain.cpp b/src/cppunit/ProtectorChain.cpp
index f528341..7e9eda6 100644
--- a/src/cppunit/ProtectorChain.cpp
+++ b/src/cppunit/ProtectorChain.cpp
@@ -21,11 +21,20 @@
   }
 
 private:
+  // disable copying
+  ProtectFunctor( const ProtectFunctor& );
+  // disable copying
+  ProtectFunctor& operator=( const ProtectFunctor& );
+
   Protector *m_protector;
   const Functor &m_functor;
   const ProtectorContext &m_context;
 };
 
+ProtectorChain::ProtectorChain()
+       : m_protectors(0)
+{
+}
 
 ProtectorChain::~ProtectorChain()
 {
diff --git a/src/cppunit/ProtectorChain.h b/src/cppunit/ProtectorChain.h
index 711b56f..1941131 100644
--- a/src/cppunit/ProtectorChain.h
+++ b/src/cppunit/ProtectorChain.h
@@ -19,6 +19,8 @@
 class CPPUNIT_API ProtectorChain : public Protector
 {
 public:
+  ProtectorChain();
+
   ~ProtectorChain();
 
   void push( Protector *protector );
diff --git a/src/cppunit/ProtectorContext.h b/src/cppunit/ProtectorContext.h
index c3d496c..4957e05 100644
--- a/src/cppunit/ProtectorContext.h
+++ b/src/cppunit/ProtectorContext.h
@@ -26,6 +26,13 @@
   {
   }
 
+private:
+  /// disable copy construction
+  ProtectorContext( const ProtectorContext& );
+  /// disable assignment
+  ProtectorContext& operator=(const ProtectorContext&);
+
+public:
   Test *m_test;
   TestResult *m_result;
   std::string m_shortDescription;
diff --git a/src/cppunit/SourceLine.cpp b/src/cppunit/SourceLine.cpp
index dfadae3..ecc9558 100644
--- a/src/cppunit/SourceLine.cpp
+++ b/src/cppunit/SourceLine.cpp
@@ -5,6 +5,7 @@
 
 
 SourceLine::SourceLine() :
+    m_fileName(),
     m_lineNumber( -1 )
 {
 }
diff --git a/src/cppunit/TestCase.cpp b/src/cppunit/TestCase.cpp
index 13c0525..10ff578 100644
--- a/src/cppunit/TestCase.cpp
+++ b/src/cppunit/TestCase.cpp
@@ -34,6 +34,10 @@
   }
 
 private:
+  // disable copying
+  TestCaseMethodFunctor( const TestCaseMethodFunctor& );
+  // disable copying
+  TestCaseMethodFunctor& operator=( const TestCaseMethodFunctor& );
   TestCase *m_target;
   Method m_method;
 };
diff --git a/src/cppunit/TestFactoryRegistry.cpp b/src/cppunit/TestFactoryRegistry.cpp
index 3457da3..4f76f66 100644
--- a/src/cppunit/TestFactoryRegistry.cpp
+++ b/src/cppunit/TestFactoryRegistry.cpp
@@ -50,6 +50,7 @@
 
 public:
   TestFactoryRegistryList()
+    : m_registries()
   {
     stateFlag( exist );
   }
@@ -83,6 +84,7 @@
 
 
 TestFactoryRegistry::TestFactoryRegistry( std::string name ) :
+    m_factories(),
     m_name( name )
 {
 }
diff --git a/src/cppunit/TestNamer.cpp b/src/cppunit/TestNamer.cpp
index eec9be9..33a3ff6 100644
--- a/src/cppunit/TestNamer.cpp
+++ b/src/cppunit/TestNamer.cpp
@@ -8,8 +8,8 @@
 
 #if CPPUNIT_HAVE_RTTI
 TestNamer::TestNamer( const std::type_info &typeInfo )
+       : m_fixtureName( TypeInfoHelper::getClassName( typeInfo ) )
 {
-  m_fixtureName = TypeInfoHelper::getClassName( typeInfo );
 }
 #endif
 
diff --git a/src/cppunit/TestPath.cpp b/src/cppunit/TestPath.cpp
index a2783a2..9d69a3c 100644
--- a/src/cppunit/TestPath.cpp
+++ b/src/cppunit/TestPath.cpp
@@ -8,11 +8,13 @@
 
 
 TestPath::TestPath()
+       : m_tests()
 {
 }
 
 
 TestPath::TestPath( Test *root )
+       : m_tests()
 {
   add( root );
 }
@@ -21,6 +23,7 @@
 TestPath::TestPath( const TestPath &other, 
                     int indexFirst, 
                     int count )
+       : m_tests()
 {
   int countAdjustment = 0;
   if ( indexFirst < 0 )
@@ -42,6 +45,7 @@
 
 TestPath::TestPath( Test *searchRoot, 
                     const std::string &pathAsString )
+       : m_tests()
 {
   PathTestNames testNames;
 
diff --git a/src/cppunit/TestResult.cpp b/src/cppunit/TestResult.cpp
index 4b02c30..ad880bc 100644
--- a/src/cppunit/TestResult.cpp
+++ b/src/cppunit/TestResult.cpp
@@ -14,7 +14,8 @@
 
 TestResult::TestResult( SynchronizationObject *syncObject )
     : SynchronizedObject( syncObject )
-    , m_protectorChain( new ProtectorChain() )
+    , m_listeners()
+    , m_protectorChain( new ProtectorChain )
     , m_stop( false )
 { 
   m_protectorChain->push( new DefaultProtector() );
diff --git a/src/cppunit/TestResultCollector.cpp b/src/cppunit/TestResultCollector.cpp
index 4371c50..bfe383a 100644
--- a/src/cppunit/TestResultCollector.cpp
+++ b/src/cppunit/TestResultCollector.cpp
@@ -7,6 +7,9 @@
 
 TestResultCollector::TestResultCollector( SynchronizationObject *syncObject )
     : TestSuccessListener( syncObject )
+       , m_tests()
+       , m_failures()
+       , m_testErrors(0)
 {
   reset();
 }
diff --git a/src/cppunit/TestSuite.cpp b/src/cppunit/TestSuite.cpp
index 8dd2ea6..14cfcd2 100644
--- a/src/cppunit/TestSuite.cpp
+++ b/src/cppunit/TestSuite.cpp
@@ -8,6 +8,7 @@
 /// Default constructor
 TestSuite::TestSuite( std::string name )
     : TestComposite( name )
+    , m_tests()
 {
 }
 
diff --git a/src/cppunit/TestSuiteBuilderContext.cpp b/src/cppunit/TestSuiteBuilderContext.cpp
index ff71b52..5e4347e 100644
--- a/src/cppunit/TestSuiteBuilderContext.cpp
+++ b/src/cppunit/TestSuiteBuilderContext.cpp
@@ -13,6 +13,7 @@
   : m_suite( suite )
   , m_namer( namer )
   , m_factory( factory )
+  , m_properties()
 {
 }
 
diff --git a/src/cppunit/XmlDocument.cpp b/src/cppunit/XmlDocument.cpp
index 31f9115..c3871ff 100644
--- a/src/cppunit/XmlDocument.cpp
+++ b/src/cppunit/XmlDocument.cpp
@@ -5,7 +5,8 @@
 
 CPPUNIT_NS_BEGIN
 
-
+// m_encoding is set in setEncoding. Ignore -Weffc++ warnings
+#pragma GCC diagnostic ignored "-Weffc++"
 XmlDocument::XmlDocument( const std::string &encoding,
                           const std::string &styleSheet )
   : m_styleSheet( styleSheet )
@@ -14,6 +15,7 @@
 {
   setEncoding( encoding );
 }
+#pragma GCC diagnostic pop
 
 
 XmlDocument::~XmlDocument()
diff --git a/src/cppunit/XmlElement.cpp b/src/cppunit/XmlElement.cpp
index f930ad4..498bc3f 100644
--- a/src/cppunit/XmlElement.cpp
+++ b/src/cppunit/XmlElement.cpp
@@ -10,6 +10,8 @@
                         std::string content ) 
   : m_name( elementName )
   , m_content( content )
+  , m_attributes()     // fix -Weffc++ warning
+  , m_elements()       // fix -Weffc++ warning
 {
 }
 
@@ -17,6 +19,9 @@
 XmlElement::XmlElement( std::string elementName,
                         int numericContent )
   : m_name( elementName )
+  , m_content()                // fix -Weffc++ warning
+  , m_attributes()     // fix -Weffc++ warning
+  , m_elements()    // fix -Weffc++ warning
 {
   setContent( numericContent );
 }
diff --git a/src/cppunit/XmlOutputter.cpp b/src/cppunit/XmlOutputter.cpp
index c605e33..145c8f4 100644
--- a/src/cppunit/XmlOutputter.cpp
+++ b/src/cppunit/XmlOutputter.cpp
@@ -15,10 +15,13 @@
 
 XmlOutputter::XmlOutputter( TestResultCollector *result,
                             OStream &stream,
-                            std::string encoding )
+                            const std::string& encoding )
   : m_result( result )
   , m_stream( stream )
+  , m_encoding( encoding )
+  , m_styleSheet() // fix -Weffc++ warning
   , m_xml( new XmlDocument( encoding ) )
+  , m_hooks() // fix -Weffc++ warning
 {
 }
 

-- 
To view, visit https://gerrit.libreoffice.org/5424
To unsubscribe, visit https://gerrit.libreoffice.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6f1cae812c58e3791c2386a1288501cf2f559610
Gerrit-PatchSet: 1
Gerrit-Project: cppunit
Gerrit-Branch: master
Gerrit-Owner: Tobias Lippert <drtl@fastmail.fm>