From d381ecc097fcf0b74765637d087c67677f2871ca Mon Sep 17 00:00:00 2001 From: Nishant Kumar Singh Date: Fri, 24 May 2019 14:04:41 +0530 Subject: [PATCH 1/3] Fix assignment operator issues in ByteArray + Fix assignment operator issues in ByteArray + Add noexcept qualifier to functions --- AnnService/inc/Core/CommonDataStructure.h | 39 ++++--- AnnService/src/Core/CommonDataStructure.cpp | 114 ++++++++------------ 2 files changed, 66 insertions(+), 87 deletions(-) diff --git a/AnnService/inc/Core/CommonDataStructure.h b/AnnService/inc/Core/CommonDataStructure.h index 74d7980f4..e03702350 100644 --- a/AnnService/inc/Core/CommonDataStructure.h +++ b/AnnService/inc/Core/CommonDataStructure.h @@ -12,35 +12,42 @@ namespace SPTAG class ByteArray { public: - ByteArray(); + const static ByteArray c_empty; - ByteArray(ByteArray&& p_right); + static ByteArray Alloc(std::size_t p_length); - ByteArray(std::uint8_t* p_array, std::size_t p_length, bool p_transferOnwership); + ByteArray() noexcept; - ByteArray(std::uint8_t* p_array, std::size_t p_length, std::shared_ptr p_dataHolder); + ByteArray(ByteArray&& p_right) noexcept; - ByteArray(const ByteArray& p_right); + ByteArray(std::uint8_t* p_array, std::size_t p_length, bool p_transferOnwership); - ByteArray& operator= (const ByteArray& p_right); + ByteArray(std::uint8_t* p_array, std::size_t p_length, std::shared_ptr p_dataHolder) noexcept; - ByteArray& operator= (ByteArray&& p_right); + ByteArray(const ByteArray& p_right) noexcept; - ~ByteArray(); + ByteArray& operator=(const ByteArray& p_right) noexcept; - static ByteArray Alloc(std::size_t p_length); + ByteArray& operator=(ByteArray&& p_right) noexcept; - std::uint8_t* Data() const; + std::uint8_t* Data() const noexcept + { + return m_data; + } - std::size_t Length() const; - - void SetData(std::uint8_t* p_array, std::size_t p_length); + std::size_t Length() const noexcept + { + return m_length; + } - std::shared_ptr DataHolder() const; + std::shared_ptr DataHolder() const noexcept + { + return m_dataHolder; + } - void Clear(); + void SetData(std::uint8_t* p_array, std::size_t p_length) noexcept; - const static ByteArray c_empty; + void Clear() noexcept; private: std::uint8_t* m_data; diff --git a/AnnService/src/Core/CommonDataStructure.cpp b/AnnService/src/Core/CommonDataStructure.cpp index 4a91554da..24d837651 100644 --- a/AnnService/src/Core/CommonDataStructure.cpp +++ b/AnnService/src/Core/CommonDataStructure.cpp @@ -3,28 +3,29 @@ #include "inc/Core/CommonDataStructure.h" -using namespace SPTAG; +namespace SPTAG +{ const ByteArray ByteArray::c_empty; -ByteArray::ByteArray() - : m_data(nullptr), - m_length(0) +ByteArray::ByteArray() noexcept + : m_data(nullptr) + , m_length(0) { } - -ByteArray::ByteArray(ByteArray&& p_right) - : m_data(p_right.m_data), - m_length(p_right.m_length), - m_dataHolder(std::move(p_right.m_dataHolder)) +ByteArray::ByteArray(ByteArray&& p_right) noexcept + : m_data(p_right.m_data) + , m_length(p_right.m_length) + , m_dataHolder(std::move(p_right.m_dataHolder)) { + p_right.m_data = nullptr; + p_right.m_length = 0; } - ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, bool p_transferOnwership) - : m_data(p_array), - m_length(p_length) + : m_data(p_array) + , m_length(p_length) { if (p_transferOnwership) { @@ -32,101 +33,72 @@ ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, bool p_transfe } } - -ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, std::shared_ptr p_dataHolder) - : m_data(p_array), - m_length(p_length), - m_dataHolder(std::move(p_dataHolder)) +ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, std::shared_ptr p_dataHolder) noexcept + : m_data(p_array) + , m_length(p_length) + , m_dataHolder(std::move(p_dataHolder)) { } - -ByteArray::ByteArray(const ByteArray& p_right) +ByteArray::ByteArray(const ByteArray& p_right) noexcept : m_data(p_right.m_data), m_length(p_right.m_length), m_dataHolder(p_right.m_dataHolder) { } - ByteArray& -ByteArray::operator= (const ByteArray& p_right) +ByteArray::operator=(const ByteArray& p_right) noexcept { - m_data = p_right.m_data; - m_length = p_right.m_length; - m_dataHolder = p_right.m_dataHolder; + if (this != std::addressof(p_right)) + { + m_data = p_right.m_data; + m_length = p_right.m_length; + m_dataHolder = p_right.m_dataHolder; + } return *this; } - ByteArray& -ByteArray::operator= (ByteArray&& p_right) +ByteArray::operator=(ByteArray&& p_right) noexcept { - m_data = p_right.m_data; - m_length = p_right.m_length; - m_dataHolder = std::move(p_right.m_dataHolder); + if (this != std::addressof(p_right)) + { + m_data = p_right.m_data; + m_length = p_right.m_length; + m_dataHolder = std::move(p_right.m_dataHolder); + } return *this; } - -ByteArray::~ByteArray() -{ -} - - ByteArray ByteArray::Alloc(std::size_t p_length) { - ByteArray byteArray; if (0 == p_length) { - return byteArray; + return ByteArray(); + } + else { + auto array = new std::uint8_t[p_length]; + return ByteArray(array, p_length, true); } - - byteArray.m_dataHolder.reset(new std::uint8_t[p_length], - std::default_delete()); - - byteArray.m_length = p_length; - byteArray.m_data = byteArray.m_dataHolder.get(); - return byteArray; -} - - -std::uint8_t* -ByteArray::Data() const -{ - return m_data; -} - - -std::size_t -ByteArray::Length() const -{ - return m_length; } - void -ByteArray::SetData(std::uint8_t* p_array, std::size_t p_length) +ByteArray::SetData(std::uint8_t* p_array, std::size_t p_length) noexcept { m_data = p_array; - m_length = p_length; -} - - -std::shared_ptr -ByteArray::DataHolder() const -{ - return m_dataHolder; + m_length = p_length; + m_dataHolder.reset(); } - void -ByteArray::Clear() +ByteArray::Clear() noexcept { m_data = nullptr; m_dataHolder.reset(); m_length = 0; -} \ No newline at end of file +} +} From 54504c80ac66ab96da39aba6d1e6591e1d29cb21 Mon Sep 17 00:00:00 2001 From: Nishant Kumar Singh Date: Thu, 30 May 2019 00:19:47 +0530 Subject: [PATCH 2/3] Revert formatting changes done to member initializer list --- AnnService/src/Core/CommonDataStructure.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/AnnService/src/Core/CommonDataStructure.cpp b/AnnService/src/Core/CommonDataStructure.cpp index 24d837651..ed8141b81 100644 --- a/AnnService/src/Core/CommonDataStructure.cpp +++ b/AnnService/src/Core/CommonDataStructure.cpp @@ -9,23 +9,23 @@ namespace SPTAG const ByteArray ByteArray::c_empty; ByteArray::ByteArray() noexcept - : m_data(nullptr) - , m_length(0) + : m_data(nullptr), + m_length(0) { } ByteArray::ByteArray(ByteArray&& p_right) noexcept - : m_data(p_right.m_data) - , m_length(p_right.m_length) - , m_dataHolder(std::move(p_right.m_dataHolder)) + : m_data(p_right.m_data), + m_length(p_right.m_length), + m_dataHolder(std::move(p_right.m_dataHolder)) { p_right.m_data = nullptr; p_right.m_length = 0; } ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, bool p_transferOnwership) - : m_data(p_array) - , m_length(p_length) + : m_data(p_array), + m_length(p_length) { if (p_transferOnwership) { @@ -34,9 +34,9 @@ ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, bool p_transfe } ByteArray::ByteArray(std::uint8_t* p_array, std::size_t p_length, std::shared_ptr p_dataHolder) noexcept - : m_data(p_array) - , m_length(p_length) - , m_dataHolder(std::move(p_dataHolder)) + : m_data(p_array), + m_length(p_length), + m_dataHolder(std::move(p_dataHolder)) { } From a019231d24a361d41a7b72e593551a50c5a5e1f5 Mon Sep 17 00:00:00 2001 From: Nishant Kumar Singh Date: Sun, 9 Jun 2019 22:52:24 +0530 Subject: [PATCH 3/3] + Remove the checks according to the review feedback --- AnnService/src/Core/CommonDataStructure.cpp | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/AnnService/src/Core/CommonDataStructure.cpp b/AnnService/src/Core/CommonDataStructure.cpp index 24d837651..448e75509 100644 --- a/AnnService/src/Core/CommonDataStructure.cpp +++ b/AnnService/src/Core/CommonDataStructure.cpp @@ -50,12 +50,9 @@ ByteArray::ByteArray(const ByteArray& p_right) noexcept ByteArray& ByteArray::operator=(const ByteArray& p_right) noexcept { - if (this != std::addressof(p_right)) - { - m_data = p_right.m_data; - m_length = p_right.m_length; - m_dataHolder = p_right.m_dataHolder; - } + m_data = p_right.m_data; + m_length = p_right.m_length; + m_dataHolder = p_right.m_dataHolder; return *this; } @@ -63,12 +60,9 @@ ByteArray::operator=(const ByteArray& p_right) noexcept ByteArray& ByteArray::operator=(ByteArray&& p_right) noexcept { - if (this != std::addressof(p_right)) - { - m_data = p_right.m_data; - m_length = p_right.m_length; - m_dataHolder = std::move(p_right.m_dataHolder); - } + m_data = p_right.m_data; + m_length = p_right.m_length; + m_dataHolder = std::move(p_right.m_dataHolder); return *this; }