Merge branch 'warnings'

Author: pavel-kirienko

libuavcan/CMakeLists.txt

@@ -119,10 +119,14 @@ if (DEBUG_BUILD)
     message(STATUS "Debug build (note: requires gtest)")
 
     if (COMPILER_IS_GCC_COMPATIBLE)
+        # No such thing as too many warnings
         set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror -pedantic -Wfloat-equal -Wconversion")
         set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wsign-conversion -Wcast-align -Wmissing-declarations -Wlogical-op")
         set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wdouble-promotion -Wswitch-enum -Wtype-limits -Wno-error=array-bounds")
-        set(cpp03_flags "-std=c++03 -Wno-variadic-macros -Wno-long-long")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wzero-as-null-pointer-constant -Wnon-virtual-dtor")
+        set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Woverloaded-virtual -Wsign-promo -Wold-style-cast")
+        #set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Weffc++ -Wno-error=effc++")  # Produces heaps of useless warnings
+        set(cpp03_flags "-std=c++03 -Wno-variadic-macros -Wno-long-long -Wno-zero-as-null-pointer-constant")
         set(optim_flags "-O3 -DNDEBUG -g0")
     else ()
         message(STATUS "Compiler ID: ${CMAKE_CXX_COMPILER_ID}")

libuavcan/dsdl_compiler/libuavcan_dsdl_compiler/data_type_template.tmpl

@@ -365,7 +365,7 @@ template <>
 inline const typename ${scope_prefix}<0>::TagToType< ${scope_prefix}<0>::Tag::${a.name} >::StorageType*
 ${scope_prefix}<0>::as< ${scope_prefix}<0>::Tag::${a.name} >() const
 {
-    return is(${scope_prefix}<0>::Tag::${a.name}) ? &${a.name} : NULL;
+    return is(${scope_prefix}<0>::Tag::${a.name}) ? &${a.name} : UAVCAN_NULLPTR;
 }
 
 template <>

libuavcan/include/uavcan/build_config.hpp

@@ -38,6 +38,18 @@
 # endif
 #endif
 
+/**
+ * The library uses UAVCAN_NULLPTR instead of UAVCAN_NULLPTR and nullptr in order to allow the use of
+ * -Wzero-as-null-pointer-constant.
+ */
+#ifndef UAVCAN_NULLPTR
+# if UAVCAN_CPP_VERSION >= UAVCAN_CPP11
+#  define UAVCAN_NULLPTR nullptr
+# else
+#  define UAVCAN_NULLPTR NULL
+# endif
+#endif
+
 /**
  * By default, libuavcan enables all features if it detects that it is being built for a general-purpose
  * target like Linux. Value of this macro influences other configuration options located below in this file.

libuavcan/include/uavcan/driver/can.hpp

@@ -46,7 +46,7 @@ struct UAVCAN_EXPORT CanFrame
         id(can_id),
         dlc((data_len > MaxDataLen) ? MaxDataLen : data_len)
     {
-        UAVCAN_ASSERT(can_data != NULL);
+        UAVCAN_ASSERT(can_data != UAVCAN_NULLPTR);
         UAVCAN_ASSERT(data_len == dlc);
         (void)copy(can_data, can_data + dlc, this->data);
     }
@@ -232,7 +232,7 @@ class UAVCAN_EXPORT ICanDriver
      * The pending TX argument contains an array of pointers to CAN frames that the library wants to transmit
      * next, per interface. This is intended to allow the driver to properly prioritize transmissions; many
      * drivers will not need to use it. If a write flag for the given interface is set to one in the select mask
-     * structure, then the corresponding pointer is guaranteed to be valid (not NULL).
+     * structure, then the corresponding pointer is guaranteed to be valid (not UAVCAN_NULLPTR).
      *
      * @param [in,out] inout_masks        Masks indicating which interfaces are needed/available for IO.
      * @param [in]     pending_tx         Array of frames, per interface, that are likely to be transmitted next.

libuavcan/include/uavcan/dynamic_memory.hpp

@@ -153,15 +153,15 @@ PoolAllocator<PoolSize, BlockSize, RaiiSynchronizer>::PoolAllocator() :
         // coverity[dead_error_line : FALSE]
         free_list_[i].next = free_list_ + i + 1;
     }
-    free_list_[NumBlocks - 1].next = NULL;
+    free_list_[NumBlocks - 1].next = UAVCAN_NULLPTR;
 }
 
 template <std::size_t PoolSize, uint8_t BlockSize, typename RaiiSynchronizer>
 void* PoolAllocator<PoolSize, BlockSize, RaiiSynchronizer>::allocate(std::size_t size)
 {
-    if (free_list_ == NULL || size > BlockSize)
+    if (free_list_ == UAVCAN_NULLPTR || size > BlockSize)
     {
-        return NULL;
+        return UAVCAN_NULLPTR;
     }
 
     RaiiSynchronizer lock;
@@ -184,7 +184,7 @@ void* PoolAllocator<PoolSize, BlockSize, RaiiSynchronizer>::allocate(std::size_t
 template <std::size_t PoolSize, uint8_t BlockSize, typename RaiiSynchronizer>
 void PoolAllocator<PoolSize, BlockSize, RaiiSynchronizer>::deallocate(const void* ptr)
 {
-    if (ptr == NULL)
+    if (ptr == UAVCAN_NULLPTR)
     {
         return;
     }

libuavcan/include/uavcan/helpers/heap_based_pool_allocator.hpp

@@ -75,7 +75,7 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator,
                                                        static_cast<uint32_t>(NumericTraits<uint16_t>::max())))),
         num_reserved_blocks_(0),
         num_allocated_blocks_(0),
-        reserve_(NULL)
+        reserve_(UAVCAN_NULLPTR)
     { }
 
     /**
@@ -101,7 +101,7 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator,
     {
         if (size > BlockSize)
         {
-            return NULL;
+            return UAVCAN_NULLPTR;
         }
 
         {
@@ -110,7 +110,7 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator,
 
             Node* const p = reserve_;
 
-            if (UAVCAN_LIKELY(p != NULL))
+            if (UAVCAN_LIKELY(p != UAVCAN_NULLPTR))
             {
                 reserve_ = reserve_->next;
                 num_allocated_blocks_++;
@@ -119,13 +119,13 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator,
 
             if (num_reserved_blocks_ >= capacity_hard_limit_)   // Hard limit reached, no further allocations
             {
-                return NULL;
+                return UAVCAN_NULLPTR;
             }
         }
 
         // Unlikely branch
         void* const m = std::malloc(sizeof(Node));
-        if (m != NULL)
+        if (m != UAVCAN_NULLPTR)
         {
             RaiiSynchronizer lock;
             (void)lock;
@@ -142,7 +142,7 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator,
      */
     virtual void deallocate(const void* ptr)
     {
-        if (ptr != NULL)
+        if (ptr != UAVCAN_NULLPTR)
         {
             RaiiSynchronizer lock;
             (void)lock;
@@ -171,15 +171,15 @@ class UAVCAN_EXPORT HeapBasedPoolAllocator : public IPoolAllocator,
      */
     void shrink()
     {
-        Node* p = NULL;
+        Node* p = UAVCAN_NULLPTR;
         for (;;)
         {
             {
                 RaiiSynchronizer lock;
                 (void)lock;
                 // Removing from reserve and updating the counter.
                 p = reserve_;
-                if (p != NULL)
+                if (p != UAVCAN_NULLPTR)
                 {
                     reserve_ = reserve_->next;
                     num_reserved_blocks_--;

libuavcan/include/uavcan/marshal/array.hpp

@@ -761,7 +761,8 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
      * Members must be comparable via operator ==.
      */
     template <typename R>
-    typename EnableIf<sizeof(((const R*)(0U))->size()) && sizeof((*((const R*)(0U)))[0]), bool>::Type
+    typename EnableIf<sizeof((reinterpret_cast<const R*>(0))->size()) &&
+                      sizeof((*(reinterpret_cast<const R*>(0)))[0]), bool>::Type
     operator==(const R& rhs) const
     {
         if (size() != rhs.size())
@@ -786,7 +787,8 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
      * Any container with size() and [] is acceptable.
      */
     template <typename R>
-    typename EnableIf<sizeof(((const R*)(0U))->size()) && sizeof((*((const R*)(0U)))[0]), bool>::Type
+    typename EnableIf<sizeof((reinterpret_cast<const R*>(0))->size()) &&
+                      sizeof((*(reinterpret_cast<const R*>(0)))[0]), bool>::Type
     isClose(const R& rhs) const
     {
         if (size() != rhs.size())
@@ -809,7 +811,7 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
      */
     bool operator==(const char* ch) const
     {
-        if (ch == NULL)
+        if (ch == UAVCAN_NULLPTR)
         {
             return false;
         }
@@ -830,7 +832,7 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
         StaticAssert<Base::IsStringLike>::check();
         StaticAssert<IsDynamic>::check();
         Base::clear();
-        if (ch == NULL)
+        if (ch == UAVCAN_NULLPTR)
         {
             handleFatalError("Array::operator=(const char*)");
         }
@@ -849,7 +851,7 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
     {
         StaticAssert<Base::IsStringLike>::check();
         StaticAssert<IsDynamic>::check();
-        if (ch == NULL)
+        if (ch == UAVCAN_NULLPTR)
         {
             handleFatalError("Array::operator+=(const char*)");
         }
@@ -1001,7 +1003,8 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
      * Note that matrix packing code uses @ref areClose() for comparison.
      */
     template <typename R>
-    typename EnableIf<sizeof(((const R*)(0U))->begin()) && sizeof(((const R*)(0U))->size())>::Type
+    typename EnableIf<sizeof((reinterpret_cast<const R*>(0))->begin()) &&
+                      sizeof((reinterpret_cast<const R*>(0))->size())>::Type
     packSquareMatrix(const R& src_row_major)
     {
         if (src_row_major.size() == MaxSize)
@@ -1057,7 +1060,8 @@ class UAVCAN_EXPORT Array : public ArrayImpl<T, ArrayMode, MaxSize_>
      * Please refer to the specification to learn more about matrix packing.
      */
     template <typename R>
-    typename EnableIf<sizeof(((const R*)(0U))->begin()) && sizeof(((const R*)(0U))->size())>::Type
+    typename EnableIf<sizeof((reinterpret_cast<const R*>(0))->begin()) &&
+                      sizeof((reinterpret_cast<const R*>(0))->size())>::Type
     unpackSquareMatrix(R& dst_row_major) const
     {
         if (dst_row_major.size() == MaxSize)

libuavcan/include/uavcan/node/generic_publisher.hpp

@@ -86,7 +86,7 @@ class UAVCAN_EXPORT GenericPublisher : public GenericPublisherBase
 {
     struct ZeroTransferBuffer : public StaticTransferBufferImpl
     {
-        ZeroTransferBuffer() : StaticTransferBufferImpl(NULL, 0) { }
+        ZeroTransferBuffer() : StaticTransferBufferImpl(UAVCAN_NULLPTR, 0) { }
     };
 
     typedef typename Select<DataStruct::MaxBitLen == 0,
@@ -138,7 +138,7 @@ class UAVCAN_EXPORT GenericPublisher : public GenericPublisherBase
     int publish(const DataStruct& message, TransferType transfer_type, NodeID dst_node_id,
                 MonotonicTime blocking_deadline = MonotonicTime())
     {
-        return genericPublish(message, transfer_type, dst_node_id, NULL, blocking_deadline);
+        return genericPublish(message, transfer_type, dst_node_id, UAVCAN_NULLPTR, blocking_deadline);
     }
 
     int publish(const DataStruct& message, TransferType transfer_type, NodeID dst_node_id, TransferID tid,

libuavcan/include/uavcan/node/generic_subscriber.hpp

@@ -29,16 +29,21 @@ namespace uavcan
  *  void first(const ReceivedDataStructure<Foo>& msg);
  *  void second(const Foo& msg);
  * In the latter case, an implicit cast will happen before the callback is invoked.
+ *
+ * This class is not copyable because it holds a reference to a stack-allocated transfer descriptor object.
+ * You can slice cast it to the underlying data type though, which would be copyable:
+ *  DataType dt = rds;  // where rds is of type ReceivedDataStructure<DataType>
+ *  // dt is now copyable
  */
 template <typename DataType_>
-class UAVCAN_EXPORT ReceivedDataStructure : public DataType_
+class UAVCAN_EXPORT ReceivedDataStructure : public DataType_, Noncopyable
 {
     const IncomingTransfer* const _transfer_;   ///< Such weird name is necessary to avoid clashing with DataType fields
 
     template <typename Ret, Ret(IncomingTransfer::*Fun) () const>
     Ret safeget() const
     {
-        if (_transfer_ == NULL)
+        if (_transfer_ == UAVCAN_NULLPTR)
         {
             return Ret();
         }
@@ -47,13 +52,13 @@ class UAVCAN_EXPORT ReceivedDataStructure : public DataType_
 
 protected:
     ReceivedDataStructure()
-        : _transfer_(NULL)
+        : _transfer_(UAVCAN_NULLPTR)
     { }
 
     ReceivedDataStructure(const IncomingTransfer* arg_transfer)
         : _transfer_(arg_transfer)
     {
-        UAVCAN_ASSERT(arg_transfer != NULL);
+        UAVCAN_ASSERT(arg_transfer != UAVCAN_NULLPTR);
     }
 
 public:
@@ -230,7 +235,7 @@ int GenericSubscriber<DataSpec, DataStruct, TransferListenerType>::checkInit()
     GlobalDataTypeRegistry::instance().freeze();
     const DataTypeDescriptor* const descr =
         GlobalDataTypeRegistry::instance().find(DataTypeKind(DataSpec::DataTypeKind), DataSpec::getDataTypeFullName());
-    if (descr == NULL)
+    if (descr == UAVCAN_NULLPTR)
     {
         UAVCAN_TRACE("GenericSubscriber", "Type [%s] is not registered", DataSpec::getDataTypeFullName());
         return -ErrUnknownDataType;

libuavcan/include/uavcan/node/global_data_type_registry.hpp

@@ -49,13 +49,13 @@ class UAVCAN_EXPORT GlobalDataTypeRegistry : Noncopyable
     {
         const DataTypeID id;
         explicit EntryInsertionComparator(const Entry* dtd)
-            : id((dtd == NULL) ? DataTypeID() : dtd->descriptor.getID())
+            : id((dtd == UAVCAN_NULLPTR) ? DataTypeID() : dtd->descriptor.getID())
         {
-            UAVCAN_ASSERT(dtd != NULL);
+            UAVCAN_ASSERT(dtd != UAVCAN_NULLPTR);
         }
         bool operator()(const Entry* entry) const
         {
-            UAVCAN_ASSERT(entry != NULL);
+            UAVCAN_ASSERT(entry != UAVCAN_NULLPTR);
             return entry->descriptor.getID() > id;
         }
     };
@@ -211,15 +211,21 @@ GlobalDataTypeRegistry::RegistrationResult GlobalDataTypeRegistry::registerDataT
     {
         return RegistrationResultFrozen;
     }
+
     static Entry entry;
+
     {
         const RegistrationResult remove_res = remove(&entry);
         if (remove_res != RegistrationResultOk)
         {
             return remove_res;
         }
     }
-    entry = Entry(DataTypeKind(Type::DataTypeKind), id, Type::getDataTypeSignature(), Type::getDataTypeFullName());
+
+    // We can't just overwrite the entry itself because it's noncopyable
+    entry.descriptor = DataTypeDescriptor(DataTypeKind(Type::DataTypeKind), id,
+                                          Type::getDataTypeSignature(), Type::getDataTypeFullName());
+
     {
         const RegistrationResult remove_res = remove(&entry);
         if (remove_res != RegistrationResultOk)