diff --git a/include/rtps/communication/PacketInfo.h b/include/rtps/communication/PacketInfo.h index 09cc50a0..240158a3 100644 --- a/include/rtps/communication/PacketInfo.h +++ b/include/rtps/communication/PacketInfo.h @@ -22,6 +22,9 @@ This file is part of embeddedRTPS. Author: i11 - Embedded Software, RWTH Aachen University */ +// Copyright 2023 Apex.AI, Inc. +// All rights reserved. + #ifndef RTPS_PACKETINFO_H #define RTPS_PACKETINFO_H @@ -45,11 +48,7 @@ struct PacketInfo { PacketInfo() = default; ~PacketInfo() = default; - PacketInfo &operator=(const PacketInfo &other) { - copyTriviallyCopyable(other); - this->buffer = other.buffer; - return *this; - } + PacketInfo &operator=(const PacketInfo &other) = delete; PacketInfo &operator=(PacketInfo &&other) noexcept { copyTriviallyCopyable(other); diff --git a/include/rtps/messages/MessageFactory.h b/include/rtps/messages/MessageFactory.h index 1b35238c..fae2f73f 100644 --- a/include/rtps/messages/MessageFactory.h +++ b/include/rtps/messages/MessageFactory.h @@ -22,6 +22,9 @@ This file is part of embeddedRTPS. Author: i11 - Embedded Software, RWTH Aachen University */ +// Copyright 2023 Apex.AI, Inc. +// All rights reserved. + #ifndef RTPS_MESSAGEFACTORY_H #define RTPS_MESSAGEFACTORY_H @@ -133,8 +136,7 @@ void addSubMessageData(Buffer &buffer, const Buffer &filledPayload, serializeMessage(buffer, msg); if (filledPayload.isValid()) { - Buffer shallowCopy = filledPayload; - buffer.append(std::move(shallowCopy)); + buffer.append(filledPayload); } } diff --git a/include/rtps/storages/PBufWrapper.h b/include/rtps/storages/PBufWrapper.h index 6e8339e5..7a5ff2a7 100644 --- a/include/rtps/storages/PBufWrapper.h +++ b/include/rtps/storages/PBufWrapper.h @@ -22,6 +22,9 @@ This file is part of embeddedRTPS. Author: i11 - Embedded Software, RWTH Aachen University */ +// Copyright 2023 Apex.AI, Inc. +// All rights reserved. + #ifndef RTPS_PBUFWRAPPER_H #define RTPS_PBUFWRAPPER_H @@ -38,28 +41,26 @@ struct PBufWrapper { explicit PBufWrapper(pbuf *bufferToWrap); explicit PBufWrapper(DataSize_t length); - // Shallow Copy. No copying of the underlying pbuf. Just another reference - // like a shared pointer. - PBufWrapper(const PBufWrapper &other); - PBufWrapper &operator=(const PBufWrapper &other); + PBufWrapper(const PBufWrapper &other) = delete; + PBufWrapper &operator=(const PBufWrapper &other) = delete; PBufWrapper(PBufWrapper &&other) noexcept; PBufWrapper &operator=(PBufWrapper &&other) noexcept; ~PBufWrapper(); - PBufWrapper deepCopy() const; - bool isValid() const; bool append(const uint8_t *data, DataSize_t length); /// Note that unused reserved memory is now part of the wrapper. New calls to /// append(uint8_t*[...]) will continue behind the appended wrapper - void append(PBufWrapper &&other); + void append(const PBufWrapper &other); bool reserve(DataSize_t length); + void destroy(); + /// After calling this function, data is added starting from the beginning /// again. It does not revert reserve. void reset(); diff --git a/src/storages/PBufWrapper.cpp b/src/storages/PBufWrapper.cpp index a99550c0..c183f049 100644 --- a/src/storages/PBufWrapper.cpp +++ b/src/storages/PBufWrapper.cpp @@ -22,6 +22,9 @@ This file is part of embeddedRTPS. Author: i11 - Embedded Software, RWTH Aachen University */ +// Copyright 2023 Apex.AI, Inc. +// All rights reserved. + #include "rtps/storages/PBufWrapper.h" #include "rtps/utils/Log.h" @@ -51,24 +54,11 @@ PBufWrapper::PBufWrapper(DataSize_t length) } } -// TODO: Uses copy assignment. Improvement possible -PBufWrapper::PBufWrapper(const PBufWrapper &other) { *this = other; } - // TODO: Uses move assignment. Improvement possible PBufWrapper::PBufWrapper(PBufWrapper &&other) noexcept { *this = std::move(other); } -PBufWrapper &PBufWrapper::operator=(const PBufWrapper &other) { - copySimpleMembersAndResetBuffer(other); - - if (other.firstElement != nullptr) { - pbuf_ref(other.firstElement); - } - firstElement = other.firstElement; - return *this; -} - PBufWrapper &PBufWrapper::operator=(PBufWrapper &&other) noexcept { copySimpleMembersAndResetBuffer(other); @@ -88,26 +78,17 @@ void PBufWrapper::copySimpleMembersAndResetBuffer(const PBufWrapper &other) { } } -PBufWrapper::~PBufWrapper() { +void PBufWrapper::destroy() +{ if (firstElement != nullptr) { pbuf_free(firstElement); + firstElement = nullptr; } + m_freeSpace = 0; } -PBufWrapper PBufWrapper::deepCopy() const { - PBufWrapper clone; - clone.copySimpleMembersAndResetBuffer(*this); - - // Decided not to use pbuf_clone because it prevents const - clone.firstElement = pbuf_alloc(m_layer, this->firstElement->tot_len, m_type); - if (clone.firstElement != nullptr) { - if (pbuf_copy(clone.firstElement, this->firstElement) != ERR_OK) { - PBUF_WRAP_LOG("PBufWrapper::deepCopy: Copy of pbuf failed"); - } - } else { - clone.m_freeSpace = 0; - } - return clone; +PBufWrapper::~PBufWrapper() { + destroy(); } bool PBufWrapper::isValid() const { return firstElement != nullptr; } @@ -131,29 +112,24 @@ bool PBufWrapper::append(const uint8_t *data, DataSize_t length) { if (err != ERR_OK) { return false; } - m_freeSpace -= length; return true; } -void PBufWrapper::append(PBufWrapper &&other) { - if (this == &other) { - return; - } +void PBufWrapper::append(const PBufWrapper &other) { if (this->firstElement == nullptr) { - *this = std::move(other); + m_freeSpace = other.m_freeSpace; + this->firstElement = other.firstElement; + pbuf_ref(this->firstElement); return; } - m_freeSpace = other.m_freeSpace; - pbuf *const newElement = other.firstElement; - pbuf_cat(this->firstElement, newElement); - - other.firstElement = nullptr; + m_freeSpace += other.m_freeSpace; + pbuf_chain(this->firstElement, other.firstElement); } bool PBufWrapper::reserve(DataSize_t length) { - auto additionalAllocation = length - m_freeSpace; + int16_t additionalAllocation = length - m_freeSpace; if (additionalAllocation <= 0) { return true; }