Merge pull request #440 from cesaref/cf

[C++] Clear up various warnings on linux/windows, add support for warnings as error
aeron-io · Nov 28, 2017 · 392456f · 392456f
2 parents aa3dde9 + 0b91ad0
commit 392456f
Show file tree

Hide file tree

Showing 14 changed files with 82 additions and 39 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -26,6 +26,9 @@ endif()
 
 option(BUILD_AERON_DRIVER "Build Aeron driver" OFF)
 
+option(C_WARNINGS_AS_ERRORS "Enable warnings as errors for C" OFF)
+option(CXX_WARNINGS_AS_ERRORS "Enable warnings as errors for C++" OFF)
+
 include(ExternalProject)
 
 project("aeron" VERSION "${VERSION_FROM_FILE}")
@@ -118,6 +121,15 @@ if(UNIX)
         set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wpedantic -Wextra -Wno-unused-parameter -std=c++11 -fexceptions -g")
         set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wpedantic -Wextra -Wno-unused-parameter -std=c11 -g")
     endif()
+
+    if(C_WARNINGS_AS_ERRORS)
+        set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Werror")
+    endif(C_WARNINGS_AS_ERRORS)
+
+    if(CXX_WARNINGS_AS_ERRORS)
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
+    endif(CXX_WARNINGS_AS_ERRORS)
+
     set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -O0")
     set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -O0")
     set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} -Ofast")
@@ -139,8 +151,8 @@ elseif(MSVC)
     add_definitions(-D_CRT_SECURE_NO_WARNINGS)
     add_definitions(-DNOMINMAX)
 
-    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /MTd /Od /Zi")
-    set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /MT")
+    set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} /MTd /Od /Zi /MP")
+    set(CMAKE_CXX_FLAGS_RELEASE "${CMAKE_CXX_FLAGS_RELEASE} /MT /MP")
 endif()
 
 ##########################################################

diff --git a/aeron-client/src/main/cpp/ControlledFragmentAssembler.h b/aeron-client/src/main/cpp/ControlledFragmentAssembler.h
@@ -96,7 +96,7 @@ class ControlledFragmentAssembler
         {
             if ((flags & FrameDescriptor::BEGIN_FRAG) == FrameDescriptor::BEGIN_FRAG)
             {
-                auto result = m_builderBySessionIdMap.emplace(header.sessionId(), m_initialBufferLength);
+                auto result = m_builderBySessionIdMap.emplace(header.sessionId(), static_cast<std::uint32_t>(m_initialBufferLength));
                 BufferBuilder& builder = result.first->second;
 
                 builder

diff --git a/aeron-client/src/main/cpp/FragmentAssembler.h b/aeron-client/src/main/cpp/FragmentAssembler.h
@@ -94,7 +94,7 @@ class FragmentAssembler
         {
             if ((flags & FrameDescriptor::BEGIN_FRAG) == FrameDescriptor::BEGIN_FRAG)
             {
-                auto result = m_builderBySessionIdMap.emplace(header.sessionId(), m_initialBufferLength);
+                auto result = m_builderBySessionIdMap.emplace(header.sessionId(), static_cast<std::uint32_t>(m_initialBufferLength));
                 BufferBuilder& builder = result.first->second;
 
                 builder

diff --git a/aeron-client/src/main/cpp/concurrent/AtomicBuffer.h b/aeron-client/src/main/cpp/concurrent/AtomicBuffer.h
@@ -40,27 +40,27 @@ class AtomicBuffer
     {
     }
 
-    AtomicBuffer(std::uint8_t *buffer, util::index_t length) :
-        m_buffer(buffer), m_length(length)
+    AtomicBuffer(std::uint8_t *buffer, size_t length) :
+        m_buffer(buffer), m_length(static_cast<util::index_t>(length))
     {
 #if !defined(DISABLE_BOUNDS_CHECKS)
-        if (AERON_COND_EXPECT(length < 0, false))
+        if (AERON_COND_EXPECT(length > std::numeric_limits<util::index_t>::max(), true))
         {
             throw aeron::util::OutOfBoundsException(
-                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %d", this, length),
+                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %lld", this, static_cast<long long>(length)),
                 SOURCEINFO);
         }
 #endif
     }
 
-    AtomicBuffer(std::uint8_t *buffer, util::index_t length, std::uint8_t initialValue) :
-        m_buffer(buffer), m_length(length)
+    AtomicBuffer(std::uint8_t *buffer, size_t length, std::uint8_t initialValue) :
+        m_buffer(buffer), m_length(static_cast<util::index_t>(length))
     {
 #if !defined(DISABLE_BOUNDS_CHECKS)
-        if (AERON_COND_EXPECT(length < 0, false))
+        if (AERON_COND_EXPECT(length > std::numeric_limits<util::index_t>::max(), true))
         {
             throw aeron::util::OutOfBoundsException(
-                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %d", this, length),
+                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %lld", this, static_cast<long long>(length)),
                 SOURCEINFO);
         }
 #endif
@@ -93,19 +93,19 @@ class AtomicBuffer
     // this class does not own the memory. It simply overlays it.
     virtual ~AtomicBuffer() = default;
 
-    inline void wrap(std::uint8_t* buffer, util::index_t length)
+    inline void wrap(std::uint8_t* buffer, size_t length)
     {
 #if !defined(DISABLE_BOUNDS_CHECKS)
-        if (AERON_COND_EXPECT(length < 0, false))
+        if (AERON_COND_EXPECT(length > std::numeric_limits<util::index_t>::max(), true))
         {
             throw aeron::util::OutOfBoundsException(
-                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %d", this, length),
+                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %lld", this, static_cast<long long>(length)),
                 SOURCEINFO);
         }
 #endif
 
         m_buffer = buffer;
-        m_length = length;
+        m_length = static_cast<util::index_t>(length);
     }
 
     inline void wrap(const AtomicBuffer& buffer)
@@ -127,18 +127,18 @@ class AtomicBuffer
         return m_length;
     }
 
-    inline void capacity(util::index_t length)
+    inline void capacity(size_t length)
     {
 #if !defined(DISABLE_BOUNDS_CHECKS)
-        if (AERON_COND_EXPECT(length < 0, false))
+        if (AERON_COND_EXPECT(length > std::numeric_limits<util::index_t>::max(), true))
         {
             throw aeron::util::OutOfBoundsException(
-                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %d", this, length),
+                aeron::util::strPrintf("Length Out of Bounds[%p]. Length: %lld", this, static_cast<long long>(length)),
                 SOURCEINFO);
         }
 #endif
 
-        m_length = length;
+        m_length = static_cast<util::index_t>(length);
     }
 
     inline std::uint8_t *buffer() const

diff --git a/aeron-client/src/main/cpp/concurrent/errors/DistinctErrorLog.h b/aeron-client/src/main/cpp/concurrent/errors/DistinctErrorLog.h
@@ -144,7 +144,7 @@ class DistinctErrorLog
         if (it == m_observations.end())
         {
             const std::string encodedError = encodeObservation(errorCode, description, message);
-            const util::index_t length = ErrorLogDescriptor::HEADER_LENGTH + encodedError.length();
+            const util::index_t length = ErrorLogDescriptor::HEADER_LENGTH + static_cast<util::index_t>(encodedError.length());
             const util::index_t offset = m_nextOffset;
 
             if ((offset + length) > m_buffer.capacity())

diff --git a/aeron-client/src/test/cpp/ExclusivePublicationTest.cpp b/aeron-client/src/test/cpp/ExclusivePublicationTest.cpp
@@ -203,7 +203,7 @@ TEST_F(ExclusivePublicationTest, shouldRotateWhenAppendTrips)
     const int nextIndex = LogBufferDescriptor::indexByTerm(TERM_ID_1, TERM_ID_1 + 1);
     EXPECT_EQ(m_logMetaDataBuffer.getInt32(LogBufferDescriptor::LOG_ACTIVE_TERM_COUNT_OFFSET), 1);
 
-    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), static_cast<std::int64_t>(TERM_ID_1 + 1) << 32);
+    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), (static_cast<std::int64_t>(TERM_ID_1 + 1)) << 32);
 
     EXPECT_GT(m_publication->offer(m_srcBuffer), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());
     EXPECT_GT(m_publication->position(), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());
@@ -225,7 +225,7 @@ TEST_F(ExclusivePublicationTest, shouldRotateWhenClaimTrips)
     const int nextIndex = LogBufferDescriptor::indexByTerm(TERM_ID_1, TERM_ID_1 + 1);
     EXPECT_EQ(m_logMetaDataBuffer.getInt32(LogBufferDescriptor::LOG_ACTIVE_TERM_COUNT_OFFSET), 1);
 
-    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), static_cast<std::int64_t>(TERM_ID_1 + 1) << 32);
+    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), (static_cast<std::int64_t>(TERM_ID_1 + 1)) << 32);
 
     EXPECT_GT(m_publication->tryClaim(SRC_BUFFER_LENGTH, bufferClaim), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());
     EXPECT_GT(m_publication->position(), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());

diff --git a/aeron-client/src/test/cpp/PublicationTest.cpp b/aeron-client/src/test/cpp/PublicationTest.cpp
@@ -190,7 +190,7 @@ TEST_F(PublicationTest, shouldRotateWhenAppendTrips)
     const int nextIndex = LogBufferDescriptor::indexByTermCount(1);
     EXPECT_EQ(m_logMetaDataBuffer.getInt32(LogBufferDescriptor::LOG_ACTIVE_TERM_COUNT_OFFSET), 1);
 
-    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), static_cast<std::int64_t>(TERM_ID_1 + 1) << 32);
+    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), (static_cast<std::int64_t>(TERM_ID_1 + 1)) << 32);
 
     EXPECT_GT(m_publication->offer(m_srcBuffer), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());
     EXPECT_GT(m_publication->position(), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());
@@ -210,7 +210,7 @@ TEST_F(PublicationTest, shouldRotateWhenClaimTrips)
     const int nextIndex = LogBufferDescriptor::indexByTermCount(1);
     EXPECT_EQ(m_logMetaDataBuffer.getInt32(LogBufferDescriptor::LOG_ACTIVE_TERM_COUNT_OFFSET), 1);
 
-    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), static_cast<std::int64_t>(TERM_ID_1 + 1) << 32);
+    EXPECT_EQ(m_logMetaDataBuffer.getInt64(termTailCounterOffset(nextIndex)), (static_cast<std::int64_t>(TERM_ID_1 + 1)) << 32);
 
     EXPECT_GT(m_publication->tryClaim(SRC_BUFFER_LENGTH, bufferClaim), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());
     EXPECT_GT(m_publication->position(), initialPosition + DataFrameHeader::LENGTH + m_srcBuffer.capacity());

diff --git a/aeron-client/src/test/cpp/concurrent/ConcurrentTest.cpp b/aeron-client/src/test/cpp/concurrent/ConcurrentTest.cpp
@@ -48,35 +48,35 @@ TEST (atomicBufferTests, checkBounds)
     });
 
     ASSERT_NO_THROW({
-        ab.putInt32(testBuffer.size() - sizeof(std::int32_t), -1);
+        ab.putInt32(convertSizeToIndex(testBuffer.size() - sizeof(std::int32_t)), -1);
     });
 
     ASSERT_NO_THROW({
-        ab.putInt64(testBuffer.size() - sizeof(std::int64_t), -1);
+        ab.putInt64(convertSizeToIndex(testBuffer.size() - sizeof(std::int64_t)), -1);
     });
 
     ASSERT_NO_THROW({
-        ab.putStringUtf8(testBuffer.size() - testString.length() - sizeof(std::int32_t), testString);
+        ab.putStringUtf8(convertSizeToIndex(testBuffer.size() - testString.length() - sizeof(std::int32_t)), testString);
     });
 
     ASSERT_THROW({
-        ab.putInt32(testBuffer.size(), -1);
+        ab.putInt32(convertSizeToIndex(testBuffer.size()), -1);
     }, OutOfBoundsException);
 
     ASSERT_THROW({
-        ab.putInt64(testBuffer.size(), -1);
+        ab.putInt64(convertSizeToIndex(testBuffer.size()), -1);
     }, OutOfBoundsException);
 
     ASSERT_THROW({
-        ab.putInt32(testBuffer.size() - sizeof(std::int32_t) + 1, -1);
+        ab.putInt32(convertSizeToIndex(testBuffer.size() - sizeof(std::int32_t) + 1), -1);
     }, OutOfBoundsException);
 
     ASSERT_THROW({
-        ab.putInt64(testBuffer.size() - sizeof(std::int64_t) + 1, -1);
+        ab.putInt64(convertSizeToIndex(testBuffer.size() - sizeof(std::int64_t) + 1), -1);
     }, OutOfBoundsException);
 
     ASSERT_THROW({
-        ab.putStringUtf8(testBuffer.size() - testString.length() - sizeof(std::int32_t) + 1, testString);
+        ab.putStringUtf8(convertSizeToIndex(testBuffer.size() - testString.length() - sizeof(std::int32_t) + 1), testString);
     }, OutOfBoundsException);
 }
 #endif

diff --git a/aeron-client/src/test/cpp/concurrent/ManyToOneRingBufferTest.cpp b/aeron-client/src/test/cpp/concurrent/ManyToOneRingBufferTest.cpp
@@ -362,7 +362,7 @@ TEST_F(ManyToOneRingBufferTest, shouldCopeWithExceptionFromHandler)
     {
         m_ringBuffer.read(handler);
     }
-    catch (const std::runtime_error& ignored)
+    catch (const std::runtime_error&)
     {
         exceptionReceived = true;
     }

diff --git a/aeron-client/src/test/cpp/concurrent/MockAtomicBuffer.cpp b/aeron-client/src/test/cpp/concurrent/MockAtomicBuffer.cpp
@@ -18,7 +18,7 @@
 
 namespace aeron { namespace concurrent { namespace mock {
 
-MockAtomicBuffer::MockAtomicBuffer(std::uint8_t *buffer, util::index_t length) :
+MockAtomicBuffer::MockAtomicBuffer(std::uint8_t *buffer, size_t length) :
     AtomicBuffer(buffer, length),
     m_realBuffer(buffer, length)
 {

diff --git a/aeron-client/src/test/cpp/concurrent/MockAtomicBuffer.h b/aeron-client/src/test/cpp/concurrent/MockAtomicBuffer.h
@@ -28,7 +28,7 @@ namespace aeron { namespace concurrent { namespace mock {
 class MockAtomicBuffer : public AtomicBuffer
 {
 public:
-    MockAtomicBuffer(std::uint8_t *buffer, util::index_t length);
+    MockAtomicBuffer(std::uint8_t *buffer, size_t length);
     virtual ~MockAtomicBuffer();
 
     MOCK_METHOD2(putUInt8, void(util::index_t offset, std::uint8_t v));

diff --git a/aeron-client/src/test/cpp/concurrent/OneToOneRingBufferTest.cpp b/aeron-client/src/test/cpp/concurrent/OneToOneRingBufferTest.cpp
@@ -363,7 +363,7 @@ TEST_F(OneToOneRingBufferTest, shouldCopeWithExceptionFromHandler)
     {
         m_ringBuffer.read(handler);
     }
-    catch (const std::runtime_error& ignored)
+    catch (const std::runtime_error&)
     {
         exceptionReceived = true;
     }

diff --git a/aeron-client/src/test/cpp/concurrent/TermAppenderTest.cpp b/aeron-client/src/test/cpp/concurrent/TermAppenderTest.cpp
@@ -130,7 +130,7 @@ TEST_F(TermAppenderTest, shouldAppendFrameTwiceToLog)
 {
     const util::index_t msgLength = 20;
     const util::index_t frameLength = DataFrameHeader::LENGTH + msgLength;
-    const std::int64_t alignedFrameLength = util::BitUtil::align(frameLength, FrameDescriptor::FRAME_ALIGNMENT);
+    const util::index_t alignedFrameLength = util::BitUtil::align(frameLength, FrameDescriptor::FRAME_ALIGNMENT);
     util::index_t tail = 0;
     testing::Sequence sequence1;
     testing::Sequence sequence2;

diff --git a/cppbuild/cppbuild b/cppbuild/cppbuild
@@ -2,6 +2,37 @@
 
 SOURCE_DIR="`pwd`"
 BUILD_DIR="`pwd`/cppbuild/Release"
+EXTRA_CMAKE_ARGS=""
+
+for option in "$@"
+do
+  case $option in
+    --c-warnings-as-errors)
+      EXTRA_CMAKE_ARGS="$EXTRA_CMAKE_ARGS -DC_WARNINGS_AS_ERRORS=ON"
+      echo "Enabling warnings as errors for c"
+      shift
+      ;;
+    --cxx-warnings-as-errors)
+      EXTRA_CMAKE_ARGS="$EXTRA_CMAKE_ARGS -DCXX_WARNINGS_AS_ERRORS=ON"
+      echo "Enabling warnings as errors for c++"
+      shift
+      ;;
+    -b|--build-aeron-driver)
+      EXTRA_CMAKE_ARGS="$EXTRA_CMAKE_ARGS -DBUILD_AERON_DRIVER=ON"
+      echo "Enabling building of aeron driver"
+      shift
+      ;;
+    -h|--help)
+      echo "$0 [--c-warnings-as-errors] [--cxx-warnings-as-errors] [--build-aeron-driver]"
+      exit
+      ;;
+    *)
+      echo "Unknown option $option"
+      echo "Use --help for help"
+      exit
+      ;;
+  esac
+done
 
 ncpus=1
 case "`uname`" in
@@ -22,4 +53,4 @@ fi
 
 mkdir -p $BUILD_DIR
 
-(cd $BUILD_DIR && cmake -G "Unix Makefiles" $SOURCE_DIR && make clean && make -j "$ncpus" all && ctest -C Release)
+(cd $BUILD_DIR && cmake -G "Unix Makefiles" $EXTRA_CMAKE_ARGS $SOURCE_DIR && make clean && make -j "$ncpus" all && ctest -C Release)