Skip to content

Commit

Permalink
Merge pull request eclipse-iceoryx#1521 from ApexAI/iox-#1196-fix-cla…
Browse files Browse the repository at this point in the history
…ng-tidy-warnings-in-relative-pointer-newtype-usage

iox-eclipse-iceoryx#1196 Fix clang-tidy warnings in `RelativePointer`
  • Loading branch information
mossmaurice authored Aug 5, 2022
2 parents 70ec584 + 12acfce commit faf1656
Show file tree
Hide file tree
Showing 19 changed files with 362 additions and 292 deletions.
17 changes: 4 additions & 13 deletions .clang-tidy-diff-scans.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
./iceoryx_hoofs/test/moduletests/test_unit*
./iceoryx_hoofs/source/units/*

./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/*
./iceoryx_hoofs/test/moduletests/test_relative_pointer*
./iceoryx_hoofs/source/relocatable_pointer/*

./iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/sofi.hpp
./iceoryx_hoofs/include/iceoryx_hoofs/internal/concurrent/sofi.inl
./iceoryx_hoofs/test/moduletests/test_concurrent_sofi.cpp
./iceoryx_hoofs/test/stresstests/test_stress_sofi.cpp
./iceoryx_hoofs/source/relocatable_pointer/relative_pointer_data.cpp
./iceoryx_hoofs/test/moduletests/test_relative_pointer_data.cpp
./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/relative_pointer_data.hpp
./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/relative_pointer_data.inl
./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/pointer_repository.inl

./iceoryx_hoofs/include/iceoryx_hoofs/posix_wrapper/*
./iceoryx_hoofs/include/iceoryx_hoofs/internal/posix_wrapper/*
Expand All @@ -26,15 +26,6 @@
./iceoryx_hoofs/source/posix_wrapper/*
./iceoryx_hoofs/source/posix_wrapper/shared_memory_object/*
./iceoryx_hoofs/test/moduletests/test_posix*
./iceoryx_hoofs/test/moduletests/test_allocator.cpp
./iceoryx_hoofs/test/moduletests/test_shared_memory.cpp
./iceoryx_hoofs/test/moduletests/test_shared_memory_object.cpp
# Will be activated with #1521
# ./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/base_relative_pointer.hpp
# ./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/relative_pointer.hpp
./iceoryx_hoofs/include/iceoryx_hoofs/internal/relocatable_pointer/relative_pointer.inl



# IMPORTANT:
# after the first # everything is considered a comment, add new files and
Expand Down
6 changes: 0 additions & 6 deletions PLANNED_FEATURES.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,3 @@ iceoryx.
* User defined memory provider to support hardware accelerators
* Man page for RouDi and the runtime
* Packages for linux distributions like debian, archlinux, gentoo

## iceoryx v2.0

* Windows support
* Request-response communication
* Service Discovery
15 changes: 8 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ Don't get too frightened of the API when strolling through the examples. Think o
"plumbing" one ("plumbing" as defined in Git, which means low-level). We're not using the "plumbing" APIs ourselves, but
instead the typed C++ API. The normal use case is that iceoryx is integrated as high-performance IPC transport layer in
a bigger framework with additional API layers.
An example for such a "porcelain" API would be [ROS 2](https://www.ros.org/). Others are listed in the next section.
An example for such a "porcelain" API would be [ROS 2](https://www.ros.org/).

You can find the full API documentation on 🌐 [https://iceoryx.io](https://iceoryx.io). <!--NOLINT explicit link to website-->

Expand Down Expand Up @@ -116,13 +116,14 @@ Please refer to the [CONTRIBUTING.md](./CONTRIBUTING.md) for a quick read-up abo

Get to know the upcoming features and the project scope in [PLANNED_FEATURES.md](./PLANNED_FEATURES.md).

## Innovations enabled by iceoryx
## Bindings and innovations enabled by iceoryx

|Name | Description | Technologies |
|---|---|---|
| [Larry.Robotics](https://gitlab.com/larry.robotics) | An iceoryx demonstrator for tinker, thinker and toddler | Demonstrator |
| [iceoryx-rs](https://github.com/eclipse-iceoryx/iceoryx-rs) | Experimental Rust wrapper for iceoryx | Rust |
| [IceRay](https://github.com/elBoberido/iceray) | An iceoryx introspection client written in Rust | Rust |
|Name | Description | Technologies |
|-------------------------------------------------------------------------------------|------------------------------------------------------------------|--------------|
| [iceoryx-rs](https://github.com/eclipse-iceoryx/iceoryx-rs) | Rust binding for iceoryx | Rust |
| [iceoryx-automotive-soa](https://github.com/eclipse-iceoryx/iceoryx-automotive-soa) | Binding for automotive frameworks like AUTOSAR Adaptive ara::com | C++ |
| [iceray](https://github.com/elBoberido/iceray) | An iceoryx introspection client written in Rust | Rust |
| [Larry.Robotics](https://gitlab.com/larry.robotics) | An iceoryx demonstrator for tinker, thinker and toddler | Demonstrator |

## Frequently Asked Questions (FAQ)

Expand Down
2 changes: 1 addition & 1 deletion iceoryx_binding_c/test/moduletests/test_listener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ TEST_F(iox_listener_test, AttachingServiceDiscoveryWithContextDataWorks)

void notifyServiceDiscovery(SubscriberPortData& portData)
{
ConditionNotifier(*portData.m_chunkReceiverData.m_conditionVariableDataPtr, 0).notify();
ConditionNotifier(*portData.m_chunkReceiverData.m_conditionVariableDataPtr.get(), 0).notify();
}

TIMING_TEST_F(iox_listener_test, NotifyingServiceDiscoveryEventWorks, Repeat(5), [&] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#ifndef IOX_HOOFS_RELOCATABLE_POINTER_BASE_RELATIVE_POINTER_HPP
#define IOX_HOOFS_RELOCATABLE_POINTER_BASE_RELATIVE_POINTER_HPP

#include "iceoryx_hoofs/cxx/newtype.hpp"
#include "iceoryx_hoofs/internal/relocatable_pointer/pointer_repository.hpp"

#include <cstdint>
Expand Down Expand Up @@ -51,28 +52,38 @@ namespace rp
class BaseRelativePointer
{
public:
using id_t = uint64_t;
struct id_t : public cxx::NewType<uint64_t,
cxx::newtype::DefaultConstructable,
cxx::newtype::CopyConstructable,
cxx::newtype::Convertable,
cxx::newtype::ConstructByValueCopy,
cxx::newtype::MoveConstructable>
{
using ThisType::ThisType;
};

using id_underlying_t = id_t::value_type;
using ptr_t = void*;
using const_ptr_t = const void* const;
using offset_t = std::uintptr_t;

/// @brief constructs a BaseRelativePointer pointing to the same pointee as ptr in a segment identified by id
/// @param[in] ptr the pointer whose pointee shall be the same for this
/// @param[in] id is the unique id of the segment
BaseRelativePointer(ptr_t ptr, id_t id) noexcept;
BaseRelativePointer(const ptr_t ptr, const id_t id) noexcept;

/// @brief constructs a BaseRelativePointer from a given offset and segment id
/// @param[in] offset is the offset
/// @param[in] id is the unique id of the segment
BaseRelativePointer(offset_t offset, id_t id) noexcept;
BaseRelativePointer(const offset_t offset, const id_t id) noexcept;

/// @brief constructs a BaseRelativePointer pointing to the same pointer as ptr
/// @param[in] ptr the pointer whose pointee shall be the same for this
BaseRelativePointer(ptr_t ptr = nullptr) noexcept;
explicit BaseRelativePointer(const ptr_t ptr = nullptr) noexcept;

/// @brief copy constructor
/// @param[in] other is the copy origin
BaseRelativePointer(const BaseRelativePointer& other) noexcept;
BaseRelativePointer(const BaseRelativePointer& other) noexcept = default;

/// @brief move constructor
/// @param[in] other is the move origin
Expand All @@ -99,7 +110,7 @@ class BaseRelativePointer

/// @brief returns the id which identifies the segment
/// @return the id which identifies the segment
id_t getId() const noexcept;
id_underlying_t getId() const noexcept;

/// @brief returns the offset
/// @return the offset
Expand All @@ -113,7 +124,7 @@ class BaseRelativePointer
/// @param[in] ptr starting address of the segment to be registered
/// @param[in] size is the size of the segment
/// @return id it was registered to
static id_t registerPtr(const ptr_t ptr, uint64_t size = 0U) noexcept;
static id_underlying_t registerPtr(const ptr_t ptr, uint64_t size = 0U) noexcept;

/// @brief tries to register a memory segment with a given size starting at ptr to a given id
/// @param[in] id is the id of the segment
Expand Down Expand Up @@ -150,7 +161,7 @@ class BaseRelativePointer
/// @brief get the id for a given ptr
/// @param[in] ptr the pointer whose corresponding id is searched for
/// @return id the pointer was registered to
static id_t searchId(ptr_t ptr) noexcept;
static id_underlying_t searchId(ptr_t ptr) noexcept;

/// @brief checks if given id is valid
/// @param[in] id is the id to be checked
Expand All @@ -159,7 +170,7 @@ class BaseRelativePointer

/// @brief returns the pointer repository
/// @return the pointer repository
static PointerRepository<id_t, ptr_t>& getRepository() noexcept;
static PointerRepository<id_underlying_t, ptr_t>& getRepository() noexcept;

/// @brief get the offset from the start address of the segment and ptr
/// @param[in] ptr is the pointer whose offset should be calculated
Expand All @@ -170,14 +181,17 @@ class BaseRelativePointer
/// @return the pointer for stored id and offset
ptr_t computeRawPtr() const noexcept;

static constexpr id_t NULL_POINTER_ID = std::numeric_limits<id_t>::max();
static constexpr id_underlying_t NULL_POINTER_ID{std::numeric_limits<id_underlying_t>::max()};
static constexpr offset_t NULL_POINTER_OFFSET = std::numeric_limits<offset_t>::max();

protected:
~BaseRelativePointer() noexcept = default;

id_t m_id{NULL_POINTER_ID};
// BaseRelativePointer only used with RelativePointer
// NOLINTBEGIN(cppcoreguidelines-non-private-member-variables-in-classes)
id_underlying_t m_id{NULL_POINTER_ID};
offset_t m_offset{NULL_POINTER_OFFSET};
// NOLINTEND(cppcoreguidelines-non-private-member-variables-in-classes)
};
} // namespace rp
} // namespace iox
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,14 @@ namespace iox
{
namespace rp
{
constexpr uint64_t MAX_POINTER_REPO_CAPACITY{10000U};

/// @brief Allows registration of memory segments with their start pointers and size.
/// This class is used to resolve relative pointers in the corresponding address space of the application.
/// Up to CAPACITY segments can be registered with MIN_ID = 1 to MAX_ID = CAPACITY - 1
/// id 0 is reserved and allows relative pointers to behave like normal pointers
/// (which is equivalent to measure the offset relative to 0).
template <typename id_t, typename ptr_t, uint64_t CAPACITY = 10000U>
template <typename id_t, typename ptr_t, uint64_t CAPACITY = MAX_POINTER_REPO_CAPACITY>
class PointerRepository
{
private:
Expand All @@ -46,8 +48,11 @@ class PointerRepository
/// @note 0 is a special purpose id and reserved
/// id 0 is reserved to interpret the offset just as a raw pointer,
/// i.e. its corresponding base ptr is 0
static constexpr size_t MIN_ID = 1U;
static constexpr size_t MAX_ID = CAPACITY - 1U;
static constexpr id_t MIN_ID{1U};
static constexpr id_t MAX_ID{CAPACITY - 1U};

static_assert(MAX_ID >= MIN_ID, "MAX_ID must be greater or equal than MIN_ID!");
static_assert(CAPACITY >= 2, "CAPACITY must be at least 2!");

public:
static constexpr id_t INVALID_ID = std::numeric_limits<id_t>::max();
Expand Down Expand Up @@ -106,7 +111,6 @@ class PointerRepository
iox::cxx::vector<Info, CAPACITY> m_info;
uint64_t m_maxRegistered{0U};
};

} // namespace rp
} // namespace iox

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ inline bool PointerRepository<id_t, ptr_t, CAPACITY>::registerPtr(id_t id, ptr_t
if (m_info[id].basePtr == nullptr)
{
m_info[id].basePtr = ptr;
// AXIVION Next Construct AutosarC++19_03-A5.2.4 : Cast is needed for pointer arithmetic and casted back to the
// original type
// AXIVION Next Construct AutosarC++19_03-A5.2.4 : Cast is needed for pointer arithmetic and casted back to
// the original type
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
m_info[id].endPtr = reinterpret_cast<ptr_t>(reinterpret_cast<uintptr_t>(ptr) + size - 1U);

if (id > m_maxRegistered)
{
m_maxRegistered = id;
Expand All @@ -61,10 +62,11 @@ inline id_t PointerRepository<id_t, ptr_t, CAPACITY>::registerPtr(const ptr_t pt
if (m_info[id].basePtr == nullptr)
{
m_info[id].basePtr = ptr;
// AXIVION Next Construct AutosarC++19_03-A5.2.4 : Cast is needed for pointer arithmetic and casted back to
// the original type
// AXIVION Next Construct AutosarC++19_03-A5.2.4 : Cast is needed for pointer arithmetic and casted back
// to the original type
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
m_info[id].endPtr = reinterpret_cast<ptr_t>(reinterpret_cast<uintptr_t>(ptr) + size - 1U);

if (id > m_maxRegistered)
{
m_maxRegistered = id;
Expand All @@ -73,7 +75,7 @@ inline id_t PointerRepository<id_t, ptr_t, CAPACITY>::registerPtr(const ptr_t pt
}
}

return INVALID_ID;
return id_t{INVALID_ID};
}

template <typename id_t, typename ptr_t, uint64_t CAPACITY>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class RelativePointer : public BaseRelativePointer

/// @brief constructs a RelativePointer pointing to the same pointee as ptr
/// @param[in] ptr the pointer whose pointee shall be the same for this
RelativePointer(ptr_t ptr) noexcept;
explicit RelativePointer(ptr_t ptr) noexcept;

/// @brief assigns the RelativePointer to point to the same pointee as ptr
/// @param[in] ptr the pointer whose pointee shall be the same for this
Expand All @@ -62,20 +62,10 @@ class RelativePointer : public BaseRelativePointer
/// @tparam U a template parameter to enable the dereferencing operator only for non-void T
/// @return a reference to the underlying object
template <typename U = T>
typename std::enable_if<!std::is_void<U>::value, U&>::type operator*() noexcept;

/// @brief access to the underlying object
/// @return a pointer to the underlying object
T* operator->() noexcept;

/// @brief dereferencing operator which returns a const reference to the underlying object
/// @tparam U a template parameter to enable the dereferencing operator only for non-void T
/// @return a const reference to the underlying object
template <typename U = T>
typename std::enable_if<!std::is_void<U>::value, const U&>::type operator*() const noexcept;

/// @brief read-only access to the underlying object
/// @return a const pointer to the underlying object
/// @return a pointer to the underlying object
T* operator->() const noexcept;

/// @brief access the underlying object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,6 @@ inline RelativePointer<T>& RelativePointer<T>::operator=(ptr_t ptr) noexcept
return *this;
}

template <typename T>
template <typename U>
inline typename std::enable_if<!std::is_void<U>::value, U&>::type RelativePointer<T>::operator*() noexcept
{
return *get();
}

template <typename T>
inline T* RelativePointer<T>::operator->() noexcept
{
return get();
}

template <typename T>
template <typename U>
inline typename std::enable_if<!std::is_void<U>::value, const U&>::type RelativePointer<T>::operator*() const noexcept
Expand Down
Loading

0 comments on commit faf1656

Please sign in to comment.