From 7dad86e2a633b55621782515395a093a44e5d75f Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 5 Dec 2024 18:02:31 +0000 Subject: [PATCH 01/15] Add CheckpointReader that prevents reading beyond a certain point in an underlying stream --- components/core/src/clp/CheckpointReader.cpp | 36 ++++++++++ components/core/src/clp/CheckpointReader.hpp | 72 ++++++++++++++++++++ 2 files changed, 108 insertions(+) create mode 100644 components/core/src/clp/CheckpointReader.cpp create mode 100644 components/core/src/clp/CheckpointReader.hpp diff --git a/components/core/src/clp/CheckpointReader.cpp b/components/core/src/clp/CheckpointReader.cpp new file mode 100644 index 000000000..5a2fda70b --- /dev/null +++ b/components/core/src/clp/CheckpointReader.cpp @@ -0,0 +1,36 @@ +#include "CheckpointReader.hpp" + +namespace clp { +ErrorCode CheckpointReader::try_seek_from_begin(size_t pos) { + m_cur_pos = pos > m_checkpoint ? m_checkpoint : pos; + auto rc = m_reader->try_seek_from_begin(m_cur_pos); + if (ErrorCode_Success != rc) { + return rc; + } + if (m_cur_pos >= m_checkpoint) { + return ErrorCode_EndOfFile; + } + return ErrorCode_Success; +} + +ErrorCode CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) { + if ((m_cur_pos + num_bytes_to_read) > m_checkpoint) { + num_bytes_to_read = m_checkpoint - m_cur_pos; + } + + if (m_cur_pos == m_checkpoint) { + return ErrorCode_EndOfFile; + } + + auto rc = m_reader->try_read(buf, num_bytes_to_read, num_bytes_read); + m_cur_pos += num_bytes_read; + if (ErrorCode_EndOfFile == rc) { + if (0 == num_bytes_read) { + return ErrorCode_EndOfFile; + } + } else if (ErrorCode_Success != rc) { + return rc; + } + return ErrorCode_Success; +} +} // namespace clp diff --git a/components/core/src/clp/CheckpointReader.hpp b/components/core/src/clp/CheckpointReader.hpp new file mode 100644 index 000000000..e72f2d85e --- /dev/null +++ b/components/core/src/clp/CheckpointReader.hpp @@ -0,0 +1,72 @@ +#ifndef CLP_CHECKPOINTREADER_HPP +#define CLP_CHECKPOINTREADER_HPP + +#include "ReaderInterface.hpp" + +namespace clp { +/** + * CheckpointReader is a ReaderInterface designed to wrap other ReaderInterfaces, and prevent users + * from reading beyond a certain point in the underlying ReaderInterface. This can help prevent + * needing to seek backwards in the underlying ReaderInterface by setting a checkpoint at the start + * of the next section to be read. + */ +class CheckpointReader : public ReaderInterface { +public: + explicit CheckpointReader(ReaderInterface* reader, size_t checkpoint) + : m_reader(reader), + m_checkpoint(checkpoint) { + if (nullptr != m_reader) { + m_cur_pos = m_reader->get_pos(); + } + if (m_cur_pos > m_checkpoint) { + throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__); + } + } + + // Methods implementing the ReaderInterface + /** + * Tries to get the current position of the read head in the underlying reader. + * @param pos Position of the read head in the underlying reader + * @return ErrorCode_Success on success + * @return ErrorCode_errno on failure + */ + ErrorCode try_get_pos(size_t& pos) override { return m_reader->try_get_pos(pos); } + + /** + * Tries to seek to the given position, limited by the checkpoint. If pos is past the checkpoint + * then this reader seeks up to the checkpoint in the underlying stream, then returns + * ErrorCode_EndOfFile on success or ErrorCode_errno otherwise. + * @param pos + * @return ErrorCode_Success on success + * @return ErrorCode_EndOfFile on EOF or if trying to seek beyond the checkpoint + * @return ErrorCode_errno on failure + */ + ErrorCode try_seek_from_begin(size_t pos) override; + + /** + * Tries to read up to a given number of bytes from the file, limited by the checkpoint. If the + * requested read would advance the read head beyond the checkpoint then the read size is + * adjusted such that the read head is advanced to the checkpoint. If a read is attempted when + * the read head is at the checkpoint then ErrorCode_EndOfFile is returned. + * @param buf + * @param num_bytes_to_read The number of bytes to try and read + * @param num_bytes_read The actual number of bytes read + * @return ErrorCode_errno on error + * @return ErrorCode_EndOfFile on EOF or trying to read after hitting checkpoint + * @return ErrorCode_Success on success + */ + ErrorCode try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) override; + + ErrorCode + try_read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) override { + return ErrorCode_Unsupported; + } + +private: + ReaderInterface* m_reader{nullptr}; + size_t m_checkpoint{}; + size_t m_cur_pos{}; +}; +} // namespace clp + +#endif // CLP_CHECKPOINTREADER_HPP From 6ac50ebd0f7e9531149d5c6cbed206730e3d38ae Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 5 Dec 2024 19:30:49 +0000 Subject: [PATCH 02/15] Fix bug in StringReader where StringReader permitted seeks beyond end of input --- components/core/src/clp/StringReader.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/core/src/clp/StringReader.cpp b/components/core/src/clp/StringReader.cpp index 9fa2c27d3..8dd0a3793 100644 --- a/components/core/src/clp/StringReader.cpp +++ b/components/core/src/clp/StringReader.cpp @@ -41,6 +41,10 @@ ErrorCode StringReader::try_read(char* buf, size_t num_bytes_to_read, size_t& nu } ErrorCode StringReader::try_seek_from_begin(size_t pos) { + if (pos > input_string.size()) { + this->pos = input_string.size(); + return ErrorCode_EndOfFile; + } this->pos = pos; return ErrorCode_Success; } From ed35cb5bf3cd6ef808dd312813d124035ac651ec Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 5 Dec 2024 19:31:37 +0000 Subject: [PATCH 03/15] Update code style of CheckpointReader to conform with standards --- components/core/src/clp/CheckpointReader.cpp | 5 +++-- components/core/src/clp/CheckpointReader.hpp | 23 ++++++++++++-------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/components/core/src/clp/CheckpointReader.cpp b/components/core/src/clp/CheckpointReader.cpp index 5a2fda70b..f17b651aa 100644 --- a/components/core/src/clp/CheckpointReader.cpp +++ b/components/core/src/clp/CheckpointReader.cpp @@ -1,7 +1,7 @@ #include "CheckpointReader.hpp" namespace clp { -ErrorCode CheckpointReader::try_seek_from_begin(size_t pos) { +auto CheckpointReader::try_seek_from_begin(size_t pos) -> ErrorCode { m_cur_pos = pos > m_checkpoint ? m_checkpoint : pos; auto rc = m_reader->try_seek_from_begin(m_cur_pos); if (ErrorCode_Success != rc) { @@ -13,7 +13,8 @@ ErrorCode CheckpointReader::try_seek_from_begin(size_t pos) { return ErrorCode_Success; } -ErrorCode CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) { +auto CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) + -> ErrorCode { if ((m_cur_pos + num_bytes_to_read) > m_checkpoint) { num_bytes_to_read = m_checkpoint - m_cur_pos; } diff --git a/components/core/src/clp/CheckpointReader.hpp b/components/core/src/clp/CheckpointReader.hpp index e72f2d85e..051e1bdcd 100644 --- a/components/core/src/clp/CheckpointReader.hpp +++ b/components/core/src/clp/CheckpointReader.hpp @@ -5,13 +5,17 @@ namespace clp { /** - * CheckpointReader is a ReaderInterface designed to wrap other ReaderInterfaces, and prevent users - * from reading beyond a certain point in the underlying ReaderInterface. This can help prevent - * needing to seek backwards in the underlying ReaderInterface by setting a checkpoint at the start - * of the next section to be read. + * CheckpointReader is a ReaderInterface designed to wrap other ReaderInterfaces and prevent users + * from reading or seeking beyond a certain point in the underlying input stream. + * + * This is useful when the underlying input stream is divided into several logical segments and we + * want to prevent a reader for an earlier segment consuming any bytes from a later segment. In + * particular, reading part of a later segment may force the reader for that later segment to seek + * backwards, which can be either inefficient or impossible for certain kinds of input streams. */ class CheckpointReader : public ReaderInterface { public: + // Constructor explicit CheckpointReader(ReaderInterface* reader, size_t checkpoint) : m_reader(reader), m_checkpoint(checkpoint) { @@ -30,7 +34,7 @@ class CheckpointReader : public ReaderInterface { * @return ErrorCode_Success on success * @return ErrorCode_errno on failure */ - ErrorCode try_get_pos(size_t& pos) override { return m_reader->try_get_pos(pos); } + auto try_get_pos(size_t& pos) -> ErrorCode override { return m_reader->try_get_pos(pos); } /** * Tries to seek to the given position, limited by the checkpoint. If pos is past the checkpoint @@ -41,7 +45,7 @@ class CheckpointReader : public ReaderInterface { * @return ErrorCode_EndOfFile on EOF or if trying to seek beyond the checkpoint * @return ErrorCode_errno on failure */ - ErrorCode try_seek_from_begin(size_t pos) override; + auto try_seek_from_begin(size_t pos) -> ErrorCode override; /** * Tries to read up to a given number of bytes from the file, limited by the checkpoint. If the @@ -55,10 +59,11 @@ class CheckpointReader : public ReaderInterface { * @return ErrorCode_EndOfFile on EOF or trying to read after hitting checkpoint * @return ErrorCode_Success on success */ - ErrorCode try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) override; + auto + try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) -> ErrorCode override; - ErrorCode - try_read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) override { + auto try_read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) + -> ErrorCode override { return ErrorCode_Unsupported; } From 34ad765fd69cf6db260ee2d2c28fe7ddcba8dc63 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 5 Dec 2024 19:32:02 +0000 Subject: [PATCH 04/15] Add tests for CheckpointReader --- components/core/CMakeLists.txt | 3 + .../core/tests/test-CheckpointReader.cpp | 94 +++++++++++++++++++ 2 files changed, 97 insertions(+) create mode 100644 components/core/tests/test-CheckpointReader.cpp diff --git a/components/core/CMakeLists.txt b/components/core/CMakeLists.txt index 193d167d8..acd9c02cf 100644 --- a/components/core/CMakeLists.txt +++ b/components/core/CMakeLists.txt @@ -353,6 +353,8 @@ set(SOURCE_FILES_unitTest src/clp/clp/run.hpp src/clp/clp/utils.cpp src/clp/clp/utils.hpp + src/clp/CheckpointReader.cpp + src/clp/CheckpointReader.hpp src/clp/CurlDownloadHandler.cpp src/clp/CurlDownloadHandler.hpp src/clp/CurlEasyHandle.hpp @@ -549,6 +551,7 @@ set(SOURCE_FILES_unitTest tests/LogSuppressor.hpp tests/test-Array.cpp tests/test-BufferedFileReader.cpp + tests/test-CheckpointReader.cpp tests/test-clp_s-end_to_end.cpp tests/test-EncodedVariableInterpreter.cpp tests/test-encoding_methods.cpp diff --git a/components/core/tests/test-CheckpointReader.cpp b/components/core/tests/test-CheckpointReader.cpp new file mode 100644 index 000000000..116759568 --- /dev/null +++ b/components/core/tests/test-CheckpointReader.cpp @@ -0,0 +1,94 @@ +#include + +#include + +#include "../src/clp/CheckpointReader.hpp" +#include "../src/clp/ErrorCode.hpp" +#include "../src/clp/StringReader.hpp" + +TEST_CASE("Test Checkpoint Reader", "[CheckpointReader]") { + constexpr char cTestString[]{"0123456789"}; + constexpr size_t cTestStringLen{std::char_traits::length(cTestString)}; + + SECTION("CheckpointReader does not support try_read_to_delimiter") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen}; + std::string tmp; + REQUIRE(clp::ErrorCode_Unsupported + == checkpoint_reader.try_read_to_delimiter('0', false, false, tmp)); + } + + SECTION("CheckpointReader does not allow reads beyond end of underlying stream.") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen + 1}; + char buf[cTestStringLen + 1]; + size_t num_bytes_read{}; + auto rc = checkpoint_reader.try_read(buf, cTestStringLen + 1, num_bytes_read); + REQUIRE(clp::ErrorCode_Success == rc); + REQUIRE(num_bytes_read == cTestStringLen); + REQUIRE(cTestStringLen == string_reader.get_pos()); + REQUIRE(cTestStringLen == checkpoint_reader.get_pos()); + } + + SECTION("CheckpointReader does not allow reads beyond checkpoint.") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, 1}; + char buf[cTestStringLen]; + size_t num_bytes_read{}; + auto rc = checkpoint_reader.try_read(buf, cTestStringLen, num_bytes_read); + REQUIRE(clp::ErrorCode_Success == rc); + REQUIRE(1 == num_bytes_read); + REQUIRE(1 == string_reader.get_pos()); + REQUIRE(1 == checkpoint_reader.get_pos()); + } + + SECTION("CheckpointReader does allow reads before checkpoint.") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, 1}; + char buf{}; + size_t num_bytes_read{}; + auto rc = checkpoint_reader.try_read(&buf, 1, num_bytes_read); + REQUIRE(clp::ErrorCode_Success == rc); + REQUIRE(1 == num_bytes_read); + REQUIRE(1 == string_reader.get_pos()); + REQUIRE(1 == checkpoint_reader.get_pos()); + } + + SECTION("CheckpointReader does not allow seeks beyond end of underlying stream.") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen + 1}; + auto rc = checkpoint_reader.try_seek_from_begin(cTestStringLen + 1); + REQUIRE(clp::ErrorCode_EndOfFile == rc); + REQUIRE(cTestStringLen == string_reader.get_pos()); + REQUIRE(cTestStringLen == checkpoint_reader.get_pos()); + } + + SECTION("CheckpointReader does not allow seeks beyond checkpoint.") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, 1}; + char buf[cTestStringLen]; + size_t num_bytes_read{}; + auto rc = checkpoint_reader.try_seek_from_begin(cTestStringLen); + REQUIRE(clp::ErrorCode_EndOfFile == rc); + REQUIRE(1 == string_reader.get_pos()); + REQUIRE(1 == checkpoint_reader.get_pos()); + } + + SECTION("CheckpointReader does allow seeks before checkpoint.") { + clp::StringReader string_reader; + string_reader.open(cTestString); + clp::CheckpointReader checkpoint_reader{&string_reader, 2}; + char buf[cTestStringLen]; + size_t num_bytes_read{}; + auto rc = checkpoint_reader.try_seek_from_begin(1); + REQUIRE(clp::ErrorCode_Success == rc); + REQUIRE(1 == string_reader.get_pos()); + REQUIRE(1 == checkpoint_reader.get_pos()); + } +} From 47b0cc08f7128e75149f4fe5902f1f247ecfd78c Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Fri, 6 Dec 2024 13:31:17 -0500 Subject: [PATCH 05/15] Apply suggestions from code review Co-authored-by: haiqi96 <14502009+haiqi96@users.noreply.github.com> --- components/core/src/clp/CheckpointReader.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/components/core/src/clp/CheckpointReader.cpp b/components/core/src/clp/CheckpointReader.cpp index f17b651aa..a93dcb50f 100644 --- a/components/core/src/clp/CheckpointReader.cpp +++ b/components/core/src/clp/CheckpointReader.cpp @@ -3,8 +3,8 @@ namespace clp { auto CheckpointReader::try_seek_from_begin(size_t pos) -> ErrorCode { m_cur_pos = pos > m_checkpoint ? m_checkpoint : pos; - auto rc = m_reader->try_seek_from_begin(m_cur_pos); - if (ErrorCode_Success != rc) { + if (auto const rc = m_reader->try_seek_from_begin(m_cur_pos); + ErrorCode_Success != rc) { return rc; } if (m_cur_pos >= m_checkpoint) { @@ -15,15 +15,15 @@ auto CheckpointReader::try_seek_from_begin(size_t pos) -> ErrorCode { auto CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) -> ErrorCode { - if ((m_cur_pos + num_bytes_to_read) > m_checkpoint) { - num_bytes_to_read = m_checkpoint - m_cur_pos; - } - if (m_cur_pos == m_checkpoint) { return ErrorCode_EndOfFile; } + + if ((m_cur_pos + num_bytes_to_read) > m_checkpoint) { + num_bytes_to_read = m_checkpoint - m_cur_pos; + } - auto rc = m_reader->try_read(buf, num_bytes_to_read, num_bytes_read); + auto const rc = m_reader->try_read(buf, num_bytes_to_read, num_bytes_read); m_cur_pos += num_bytes_read; if (ErrorCode_EndOfFile == rc) { if (0 == num_bytes_read) { From 7aff0c662c693c5ad0531f3d33e3388fc5fd410f Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 6 Dec 2024 18:43:27 +0000 Subject: [PATCH 06/15] Address review comments --- components/core/src/clp/CheckpointReader.cpp | 5 ++--- components/core/src/clp/CheckpointReader.hpp | 5 +++-- components/core/tests/test-CheckpointReader.cpp | 5 +++++ 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/CheckpointReader.cpp b/components/core/src/clp/CheckpointReader.cpp index a93dcb50f..4e2fdd606 100644 --- a/components/core/src/clp/CheckpointReader.cpp +++ b/components/core/src/clp/CheckpointReader.cpp @@ -3,8 +3,7 @@ namespace clp { auto CheckpointReader::try_seek_from_begin(size_t pos) -> ErrorCode { m_cur_pos = pos > m_checkpoint ? m_checkpoint : pos; - if (auto const rc = m_reader->try_seek_from_begin(m_cur_pos); - ErrorCode_Success != rc) { + if (auto const rc = m_reader->try_seek_from_begin(m_cur_pos); ErrorCode_Success != rc) { return rc; } if (m_cur_pos >= m_checkpoint) { @@ -18,7 +17,7 @@ auto CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num if (m_cur_pos == m_checkpoint) { return ErrorCode_EndOfFile; } - + if ((m_cur_pos + num_bytes_to_read) > m_checkpoint) { num_bytes_to_read = m_checkpoint - m_cur_pos; } diff --git a/components/core/src/clp/CheckpointReader.hpp b/components/core/src/clp/CheckpointReader.hpp index 051e1bdcd..c90801862 100644 --- a/components/core/src/clp/CheckpointReader.hpp +++ b/components/core/src/clp/CheckpointReader.hpp @@ -19,9 +19,10 @@ class CheckpointReader : public ReaderInterface { explicit CheckpointReader(ReaderInterface* reader, size_t checkpoint) : m_reader(reader), m_checkpoint(checkpoint) { - if (nullptr != m_reader) { - m_cur_pos = m_reader->get_pos(); + if (nullptr == m_reader) { + throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__); } + m_cur_pos = m_reader->get_pos(); if (m_cur_pos > m_checkpoint) { throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__); } diff --git a/components/core/tests/test-CheckpointReader.cpp b/components/core/tests/test-CheckpointReader.cpp index 116759568..216a423a0 100644 --- a/components/core/tests/test-CheckpointReader.cpp +++ b/components/core/tests/test-CheckpointReader.cpp @@ -43,6 +43,11 @@ TEST_CASE("Test Checkpoint Reader", "[CheckpointReader]") { REQUIRE(1 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); REQUIRE(1 == checkpoint_reader.get_pos()); + rc = checkpoint_reader.try_read(buf, 1, num_bytes_read); + REQUIRE(clp::ErrorCode_EndOfFile == rc); + REQUIRE(0 == num_bytes_read); + REQUIRE(1 == string_reader.get_pos()); + REQUIRE(1 == checkpoint_reader.get_pos()); } SECTION("CheckpointReader does allow reads before checkpoint.") { From 2ab88fdd645c10aaae1c65475c74b077498b39bc Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 6 Dec 2024 18:47:07 +0000 Subject: [PATCH 07/15] Ensure CheckpointReader sets num_bytes_read to zero on EOF --- components/core/src/clp/CheckpointReader.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/components/core/src/clp/CheckpointReader.cpp b/components/core/src/clp/CheckpointReader.cpp index 4e2fdd606..20a1472c3 100644 --- a/components/core/src/clp/CheckpointReader.cpp +++ b/components/core/src/clp/CheckpointReader.cpp @@ -15,6 +15,7 @@ auto CheckpointReader::try_seek_from_begin(size_t pos) -> ErrorCode { auto CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) -> ErrorCode { if (m_cur_pos == m_checkpoint) { + num_bytes_read = 0; return ErrorCode_EndOfFile; } From 8977dc4393258ee5e3476507a8afaf44fb2aaee2 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Sat, 7 Dec 2024 15:14:13 +0000 Subject: [PATCH 08/15] Rename to BoundedReader, rename to m_curr_pos, only update curr_pos on error if EOF --- components/core/CMakeLists.txt | 6 +- components/core/src/clp/BoundedReader.cpp | 39 ++++++++++++ ...CheckpointReader.hpp => BoundedReader.hpp} | 22 +++---- components/core/src/clp/CheckpointReader.cpp | 37 ----------- ...pointReader.cpp => test-BoundedReader.cpp} | 62 +++++++++---------- 5 files changed, 84 insertions(+), 82 deletions(-) create mode 100644 components/core/src/clp/BoundedReader.cpp rename components/core/src/clp/{CheckpointReader.hpp => BoundedReader.hpp} (85%) delete mode 100644 components/core/src/clp/CheckpointReader.cpp rename components/core/tests/{test-CheckpointReader.cpp => test-BoundedReader.cpp} (50%) diff --git a/components/core/CMakeLists.txt b/components/core/CMakeLists.txt index acd9c02cf..27792087c 100644 --- a/components/core/CMakeLists.txt +++ b/components/core/CMakeLists.txt @@ -334,6 +334,8 @@ set(SOURCE_FILES_unitTest src/clp/aws/AwsAuthenticationSigner.cpp src/clp/aws/AwsAuthenticationSigner.hpp src/clp/aws/constants.hpp + src/clp/BoundedReader.cpp + src/clp/BoundedReader.hpp src/clp/BufferedFileReader.cpp src/clp/BufferedFileReader.hpp src/clp/BufferReader.cpp @@ -353,8 +355,6 @@ set(SOURCE_FILES_unitTest src/clp/clp/run.hpp src/clp/clp/utils.cpp src/clp/clp/utils.hpp - src/clp/CheckpointReader.cpp - src/clp/CheckpointReader.hpp src/clp/CurlDownloadHandler.cpp src/clp/CurlDownloadHandler.hpp src/clp/CurlEasyHandle.hpp @@ -550,8 +550,8 @@ set(SOURCE_FILES_unitTest submodules/sqlite3/sqlite3ext.h tests/LogSuppressor.hpp tests/test-Array.cpp + tests/test-BoundedReader.cpp tests/test-BufferedFileReader.cpp - tests/test-CheckpointReader.cpp tests/test-clp_s-end_to_end.cpp tests/test-EncodedVariableInterpreter.cpp tests/test-encoding_methods.cpp diff --git a/components/core/src/clp/BoundedReader.cpp b/components/core/src/clp/BoundedReader.cpp new file mode 100644 index 000000000..282ba03c3 --- /dev/null +++ b/components/core/src/clp/BoundedReader.cpp @@ -0,0 +1,39 @@ +#include "BoundedReader.hpp" + +namespace clp { +auto BoundedReader::try_seek_from_begin(size_t pos) -> ErrorCode { + auto next_pos = pos > m_bound ? m_bound : pos; + if (auto const rc = m_reader->try_seek_from_begin(next_pos); ErrorCode_Success != rc) { + m_curr_pos = ErrorCode_EndOfFile == rc ? next_pos : m_curr_pos; + return rc; + } + m_curr_pos = next_pos; + if (m_curr_pos >= m_bound) { + return ErrorCode_EndOfFile; + } + return ErrorCode_Success; +} + +auto BoundedReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) + -> ErrorCode { + if (m_curr_pos == m_bound) { + num_bytes_read = 0; + return ErrorCode_EndOfFile; + } + + if ((m_curr_pos + num_bytes_to_read) > m_bound) { + num_bytes_to_read = m_bound - m_curr_pos; + } + + auto const rc = m_reader->try_read(buf, num_bytes_to_read, num_bytes_read); + m_curr_pos += num_bytes_read; + if (ErrorCode_EndOfFile == rc) { + if (0 == num_bytes_read) { + return ErrorCode_EndOfFile; + } + } else if (ErrorCode_Success != rc) { + return rc; + } + return ErrorCode_Success; +} +} // namespace clp diff --git a/components/core/src/clp/CheckpointReader.hpp b/components/core/src/clp/BoundedReader.hpp similarity index 85% rename from components/core/src/clp/CheckpointReader.hpp rename to components/core/src/clp/BoundedReader.hpp index c90801862..4c78186dd 100644 --- a/components/core/src/clp/CheckpointReader.hpp +++ b/components/core/src/clp/BoundedReader.hpp @@ -1,11 +1,11 @@ -#ifndef CLP_CHECKPOINTREADER_HPP -#define CLP_CHECKPOINTREADER_HPP +#ifndef CLP_BOUNDEDREADER_HPP +#define CLP_BOUNDEDREADER_HPP #include "ReaderInterface.hpp" namespace clp { /** - * CheckpointReader is a ReaderInterface designed to wrap other ReaderInterfaces and prevent users + * BoundedReader is a ReaderInterface designed to wrap other ReaderInterfaces and prevent users * from reading or seeking beyond a certain point in the underlying input stream. * * This is useful when the underlying input stream is divided into several logical segments and we @@ -13,17 +13,17 @@ namespace clp { * particular, reading part of a later segment may force the reader for that later segment to seek * backwards, which can be either inefficient or impossible for certain kinds of input streams. */ -class CheckpointReader : public ReaderInterface { +class BoundedReader : public ReaderInterface { public: // Constructor - explicit CheckpointReader(ReaderInterface* reader, size_t checkpoint) + explicit BoundedReader(ReaderInterface* reader, size_t bound) : m_reader(reader), - m_checkpoint(checkpoint) { + m_bound(bound) { if (nullptr == m_reader) { throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__); } - m_cur_pos = m_reader->get_pos(); - if (m_cur_pos > m_checkpoint) { + m_curr_pos = m_reader->get_pos(); + if (m_curr_pos > m_bound) { throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__); } } @@ -70,9 +70,9 @@ class CheckpointReader : public ReaderInterface { private: ReaderInterface* m_reader{nullptr}; - size_t m_checkpoint{}; - size_t m_cur_pos{}; + size_t m_bound{}; + size_t m_curr_pos{}; }; } // namespace clp -#endif // CLP_CHECKPOINTREADER_HPP +#endif // CLP_BOUNDEDREADER_HPP diff --git a/components/core/src/clp/CheckpointReader.cpp b/components/core/src/clp/CheckpointReader.cpp deleted file mode 100644 index 20a1472c3..000000000 --- a/components/core/src/clp/CheckpointReader.cpp +++ /dev/null @@ -1,37 +0,0 @@ -#include "CheckpointReader.hpp" - -namespace clp { -auto CheckpointReader::try_seek_from_begin(size_t pos) -> ErrorCode { - m_cur_pos = pos > m_checkpoint ? m_checkpoint : pos; - if (auto const rc = m_reader->try_seek_from_begin(m_cur_pos); ErrorCode_Success != rc) { - return rc; - } - if (m_cur_pos >= m_checkpoint) { - return ErrorCode_EndOfFile; - } - return ErrorCode_Success; -} - -auto CheckpointReader::try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) - -> ErrorCode { - if (m_cur_pos == m_checkpoint) { - num_bytes_read = 0; - return ErrorCode_EndOfFile; - } - - if ((m_cur_pos + num_bytes_to_read) > m_checkpoint) { - num_bytes_to_read = m_checkpoint - m_cur_pos; - } - - auto const rc = m_reader->try_read(buf, num_bytes_to_read, num_bytes_read); - m_cur_pos += num_bytes_read; - if (ErrorCode_EndOfFile == rc) { - if (0 == num_bytes_read) { - return ErrorCode_EndOfFile; - } - } else if (ErrorCode_Success != rc) { - return rc; - } - return ErrorCode_Success; -} -} // namespace clp diff --git a/components/core/tests/test-CheckpointReader.cpp b/components/core/tests/test-BoundedReader.cpp similarity index 50% rename from components/core/tests/test-CheckpointReader.cpp rename to components/core/tests/test-BoundedReader.cpp index 216a423a0..3cf4a8405 100644 --- a/components/core/tests/test-CheckpointReader.cpp +++ b/components/core/tests/test-BoundedReader.cpp @@ -2,98 +2,98 @@ #include -#include "../src/clp/CheckpointReader.hpp" +#include "../src/clp/BoundedReader.hpp" #include "../src/clp/ErrorCode.hpp" #include "../src/clp/StringReader.hpp" -TEST_CASE("Test Checkpoint Reader", "[CheckpointReader]") { +TEST_CASE("Test Bounded Reader", "[BoundedReader]") { constexpr char cTestString[]{"0123456789"}; constexpr size_t cTestStringLen{std::char_traits::length(cTestString)}; - SECTION("CheckpointReader does not support try_read_to_delimiter") { + SECTION("BoundedReader does not support try_read_to_delimiter") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen}; + clp::BoundedReader bounded_reader{&string_reader, cTestStringLen}; std::string tmp; REQUIRE(clp::ErrorCode_Unsupported - == checkpoint_reader.try_read_to_delimiter('0', false, false, tmp)); + == bounded_reader.try_read_to_delimiter('0', false, false, tmp)); } - SECTION("CheckpointReader does not allow reads beyond end of underlying stream.") { + SECTION("BoundedReader does not allow reads beyond end of underlying stream.") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen + 1}; + clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1}; char buf[cTestStringLen + 1]; size_t num_bytes_read{}; - auto rc = checkpoint_reader.try_read(buf, cTestStringLen + 1, num_bytes_read); + auto rc = bounded_reader.try_read(buf, cTestStringLen + 1, num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(num_bytes_read == cTestStringLen); REQUIRE(cTestStringLen == string_reader.get_pos()); - REQUIRE(cTestStringLen == checkpoint_reader.get_pos()); + REQUIRE(cTestStringLen == bounded_reader.get_pos()); } - SECTION("CheckpointReader does not allow reads beyond checkpoint.") { + SECTION("BoundedReader does not allow reads beyond checkpoint.") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, 1}; + clp::BoundedReader bounded_reader{&string_reader, 1}; char buf[cTestStringLen]; size_t num_bytes_read{}; - auto rc = checkpoint_reader.try_read(buf, cTestStringLen, num_bytes_read); + auto rc = bounded_reader.try_read(buf, cTestStringLen, num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(1 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); - REQUIRE(1 == checkpoint_reader.get_pos()); - rc = checkpoint_reader.try_read(buf, 1, num_bytes_read); + REQUIRE(1 == bounded_reader.get_pos()); + rc = bounded_reader.try_read(buf, 1, num_bytes_read); REQUIRE(clp::ErrorCode_EndOfFile == rc); REQUIRE(0 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); - REQUIRE(1 == checkpoint_reader.get_pos()); + REQUIRE(1 == bounded_reader.get_pos()); } - SECTION("CheckpointReader does allow reads before checkpoint.") { + SECTION("BoundedReader does allow reads before checkpoint.") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, 1}; + clp::BoundedReader bounded_reader{&string_reader, 1}; char buf{}; size_t num_bytes_read{}; - auto rc = checkpoint_reader.try_read(&buf, 1, num_bytes_read); + auto rc = bounded_reader.try_read(&buf, 1, num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(1 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); - REQUIRE(1 == checkpoint_reader.get_pos()); + REQUIRE(1 == bounded_reader.get_pos()); } - SECTION("CheckpointReader does not allow seeks beyond end of underlying stream.") { + SECTION("BoundedReader does not allow seeks beyond end of underlying stream.") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, cTestStringLen + 1}; - auto rc = checkpoint_reader.try_seek_from_begin(cTestStringLen + 1); + clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1}; + auto rc = bounded_reader.try_seek_from_begin(cTestStringLen + 1); REQUIRE(clp::ErrorCode_EndOfFile == rc); REQUIRE(cTestStringLen == string_reader.get_pos()); - REQUIRE(cTestStringLen == checkpoint_reader.get_pos()); + REQUIRE(cTestStringLen == bounded_reader.get_pos()); } - SECTION("CheckpointReader does not allow seeks beyond checkpoint.") { + SECTION("BoundedReader does not allow seeks beyond checkpoint.") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, 1}; + clp::BoundedReader bounded_reader{&string_reader, 1}; char buf[cTestStringLen]; size_t num_bytes_read{}; - auto rc = checkpoint_reader.try_seek_from_begin(cTestStringLen); + auto rc = bounded_reader.try_seek_from_begin(cTestStringLen); REQUIRE(clp::ErrorCode_EndOfFile == rc); REQUIRE(1 == string_reader.get_pos()); - REQUIRE(1 == checkpoint_reader.get_pos()); + REQUIRE(1 == bounded_reader.get_pos()); } - SECTION("CheckpointReader does allow seeks before checkpoint.") { + SECTION("BoundedReader does allow seeks before checkpoint.") { clp::StringReader string_reader; string_reader.open(cTestString); - clp::CheckpointReader checkpoint_reader{&string_reader, 2}; + clp::BoundedReader bounded_reader{&string_reader, 2}; char buf[cTestStringLen]; size_t num_bytes_read{}; - auto rc = checkpoint_reader.try_seek_from_begin(1); + auto rc = bounded_reader.try_seek_from_begin(1); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(1 == string_reader.get_pos()); - REQUIRE(1 == checkpoint_reader.get_pos()); + REQUIRE(1 == bounded_reader.get_pos()); } } From 93c7d3bf4214cae061fe0e7b9de3664089856e5b Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Sun, 8 Dec 2024 11:05:20 -0500 Subject: [PATCH 09/15] Update components/core/src/clp/BoundedReader.cpp Co-authored-by: haiqi96 <14502009+haiqi96@users.noreply.github.com> --- components/core/src/clp/BoundedReader.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/BoundedReader.cpp b/components/core/src/clp/BoundedReader.cpp index 282ba03c3..d04e65976 100644 --- a/components/core/src/clp/BoundedReader.cpp +++ b/components/core/src/clp/BoundedReader.cpp @@ -2,7 +2,7 @@ namespace clp { auto BoundedReader::try_seek_from_begin(size_t pos) -> ErrorCode { - auto next_pos = pos > m_bound ? m_bound : pos; + auto const next_pos = pos > m_bound ? m_bound : pos; if (auto const rc = m_reader->try_seek_from_begin(next_pos); ErrorCode_Success != rc) { m_curr_pos = ErrorCode_EndOfFile == rc ? next_pos : m_curr_pos; return rc; From 6c3a537a7a2ace9abf68b664e9308b1e94dc44d6 Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Tue, 10 Dec 2024 11:29:48 -0500 Subject: [PATCH 10/15] Apply suggestions from code review Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> --- components/core/src/clp/BoundedReader.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/core/src/clp/BoundedReader.hpp b/components/core/src/clp/BoundedReader.hpp index 4c78186dd..4f56e811a 100644 --- a/components/core/src/clp/BoundedReader.hpp +++ b/components/core/src/clp/BoundedReader.hpp @@ -17,8 +17,8 @@ class BoundedReader : public ReaderInterface { public: // Constructor explicit BoundedReader(ReaderInterface* reader, size_t bound) - : m_reader(reader), - m_bound(bound) { + : m_reader{reader}, + m_bound{bound} { if (nullptr == m_reader) { throw ReaderInterface::OperationFailed(ErrorCode_BadParam, __FILE__, __LINE__); } @@ -31,7 +31,7 @@ class BoundedReader : public ReaderInterface { // Methods implementing the ReaderInterface /** * Tries to get the current position of the read head in the underlying reader. - * @param pos Position of the read head in the underlying reader + * @param pos Returns the position of the underlying reader's head * @return ErrorCode_Success on success * @return ErrorCode_errno on failure */ From 6321c0c4e02a12f0e73b4becf34d0e0dcb60dfbb Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Tue, 10 Dec 2024 11:42:56 -0500 Subject: [PATCH 11/15] Apply suggestions from code review Co-authored-by: Lin Zhihao <59785146+LinZhihao-723@users.noreply.github.com> --- components/core/src/clp/BoundedReader.hpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/components/core/src/clp/BoundedReader.hpp b/components/core/src/clp/BoundedReader.hpp index 4f56e811a..d2b857c3d 100644 --- a/components/core/src/clp/BoundedReader.hpp +++ b/components/core/src/clp/BoundedReader.hpp @@ -38,9 +38,7 @@ class BoundedReader : public ReaderInterface { auto try_get_pos(size_t& pos) -> ErrorCode override { return m_reader->try_get_pos(pos); } /** - * Tries to seek to the given position, limited by the checkpoint. If pos is past the checkpoint - * then this reader seeks up to the checkpoint in the underlying stream, then returns - * ErrorCode_EndOfFile on success or ErrorCode_errno otherwise. + * Tries to seek to the given position, limited by the bound. * @param pos * @return ErrorCode_Success on success * @return ErrorCode_EndOfFile on EOF or if trying to seek beyond the checkpoint @@ -49,10 +47,7 @@ class BoundedReader : public ReaderInterface { auto try_seek_from_begin(size_t pos) -> ErrorCode override; /** - * Tries to read up to a given number of bytes from the file, limited by the checkpoint. If the - * requested read would advance the read head beyond the checkpoint then the read size is - * adjusted such that the read head is advanced to the checkpoint. If a read is attempted when - * the read head is at the checkpoint then ErrorCode_EndOfFile is returned. + * Tries to read up to a given number of bytes from the file, limited by the bound. * @param buf * @param num_bytes_to_read The number of bytes to try and read * @param num_bytes_read The actual number of bytes read From e54d434ff366f94e366b2149de183b0954eeb933 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Tue, 10 Dec 2024 16:53:53 +0000 Subject: [PATCH 12/15] Address review comments --- components/core/src/clp/BoundedReader.hpp | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/BoundedReader.hpp b/components/core/src/clp/BoundedReader.hpp index d2b857c3d..f6abb3abb 100644 --- a/components/core/src/clp/BoundedReader.hpp +++ b/components/core/src/clp/BoundedReader.hpp @@ -35,7 +35,9 @@ class BoundedReader : public ReaderInterface { * @return ErrorCode_Success on success * @return ErrorCode_errno on failure */ - auto try_get_pos(size_t& pos) -> ErrorCode override { return m_reader->try_get_pos(pos); } + [[nodiscard]] auto try_get_pos(size_t& pos) -> ErrorCode override { + return m_reader->try_get_pos(pos); + } /** * Tries to seek to the given position, limited by the bound. @@ -44,7 +46,7 @@ class BoundedReader : public ReaderInterface { * @return ErrorCode_EndOfFile on EOF or if trying to seek beyond the checkpoint * @return ErrorCode_errno on failure */ - auto try_seek_from_begin(size_t pos) -> ErrorCode override; + [[nodicard]] auto try_seek_from_begin(size_t pos) -> ErrorCode override; /** * Tries to read up to a given number of bytes from the file, limited by the bound. @@ -55,11 +57,21 @@ class BoundedReader : public ReaderInterface { * @return ErrorCode_EndOfFile on EOF or trying to read after hitting checkpoint * @return ErrorCode_Success on success */ - auto + [[nodiscard]] auto try_read(char* buf, size_t num_bytes_to_read, size_t& num_bytes_read) -> ErrorCode override; - auto try_read_to_delimiter(char delim, bool keep_delimiter, bool append, std::string& str) - -> ErrorCode override { + /** + * This function is unsupported because BoundedReader can not delegate to a potentially + * efficient implementation in the underlying reader, as the underlying reader's implementation + * will not respect the bound. + * @return ErrorCode_Unsupported + */ + [[nodicard]] auto try_read_to_delimiter( + [[maybe_unused]] char delim, + [[maybe_unused]] bool keep_delimiter, + [[maybe_unused]] bool append, + [[maybe_unused]] std::string& str + ) -> ErrorCode override { return ErrorCode_Unsupported; } From cbe20731f3bdf769b4162ca71c4ae6c74550b4cc Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Wed, 11 Dec 2024 21:08:47 +0000 Subject: [PATCH 13/15] Fix clang-tidy warnings --- components/core/src/clp/BoundedReader.cpp | 4 ++++ components/core/src/clp/BoundedReader.hpp | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/components/core/src/clp/BoundedReader.cpp b/components/core/src/clp/BoundedReader.cpp index d04e65976..9bca08f71 100644 --- a/components/core/src/clp/BoundedReader.cpp +++ b/components/core/src/clp/BoundedReader.cpp @@ -1,5 +1,9 @@ #include "BoundedReader.hpp" +#include + +#include "ErrorCode.hpp" + namespace clp { auto BoundedReader::try_seek_from_begin(size_t pos) -> ErrorCode { auto const next_pos = pos > m_bound ? m_bound : pos; diff --git a/components/core/src/clp/BoundedReader.hpp b/components/core/src/clp/BoundedReader.hpp index f6abb3abb..cfcb07422 100644 --- a/components/core/src/clp/BoundedReader.hpp +++ b/components/core/src/clp/BoundedReader.hpp @@ -1,6 +1,10 @@ #ifndef CLP_BOUNDEDREADER_HPP #define CLP_BOUNDEDREADER_HPP +#include +#include + +#include "ErrorCode.hpp" #include "ReaderInterface.hpp" namespace clp { @@ -46,7 +50,7 @@ class BoundedReader : public ReaderInterface { * @return ErrorCode_EndOfFile on EOF or if trying to seek beyond the checkpoint * @return ErrorCode_errno on failure */ - [[nodicard]] auto try_seek_from_begin(size_t pos) -> ErrorCode override; + [[nodiscard]] auto try_seek_from_begin(size_t pos) -> ErrorCode override; /** * Tries to read up to a given number of bytes from the file, limited by the bound. @@ -66,7 +70,7 @@ class BoundedReader : public ReaderInterface { * will not respect the bound. * @return ErrorCode_Unsupported */ - [[nodicard]] auto try_read_to_delimiter( + [[nodiscard]] auto try_read_to_delimiter( [[maybe_unused]] char delim, [[maybe_unused]] bool keep_delimiter, [[maybe_unused]] bool append, From 4dcb7ff26c4f9a703d7002bc3c411e4c7a4a1f66 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 13 Dec 2024 15:49:49 +0000 Subject: [PATCH 14/15] Minor updates to test-BoundedReader.cpp --- components/core/tests/test-BoundedReader.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/components/core/tests/test-BoundedReader.cpp b/components/core/tests/test-BoundedReader.cpp index 3cf4a8405..de489b655 100644 --- a/components/core/tests/test-BoundedReader.cpp +++ b/components/core/tests/test-BoundedReader.cpp @@ -1,3 +1,5 @@ +#include +#include #include #include @@ -23,9 +25,9 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { clp::StringReader string_reader; string_reader.open(cTestString); clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1}; - char buf[cTestStringLen + 1]; + std::array buf{}; size_t num_bytes_read{}; - auto rc = bounded_reader.try_read(buf, cTestStringLen + 1, num_bytes_read); + auto rc = bounded_reader.try_read(buf.data(), cTestStringLen + 1, num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(num_bytes_read == cTestStringLen); REQUIRE(cTestStringLen == string_reader.get_pos()); @@ -36,14 +38,14 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { clp::StringReader string_reader; string_reader.open(cTestString); clp::BoundedReader bounded_reader{&string_reader, 1}; - char buf[cTestStringLen]; + std::array buf{}; size_t num_bytes_read{}; - auto rc = bounded_reader.try_read(buf, cTestStringLen, num_bytes_read); + auto rc = bounded_reader.try_read(buf.data(), cTestStringLen, num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(1 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); REQUIRE(1 == bounded_reader.get_pos()); - rc = bounded_reader.try_read(buf, 1, num_bytes_read); + rc = bounded_reader.try_read(buf.data(), 1, num_bytes_read); REQUIRE(clp::ErrorCode_EndOfFile == rc); REQUIRE(0 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); @@ -77,7 +79,6 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { clp::StringReader string_reader; string_reader.open(cTestString); clp::BoundedReader bounded_reader{&string_reader, 1}; - char buf[cTestStringLen]; size_t num_bytes_read{}; auto rc = bounded_reader.try_seek_from_begin(cTestStringLen); REQUIRE(clp::ErrorCode_EndOfFile == rc); @@ -89,7 +90,6 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { clp::StringReader string_reader; string_reader.open(cTestString); clp::BoundedReader bounded_reader{&string_reader, 2}; - char buf[cTestStringLen]; size_t num_bytes_read{}; auto rc = bounded_reader.try_seek_from_begin(1); REQUIRE(clp::ErrorCode_Success == rc); From d261ca24a30dbd13e7db4614718e90fddd5e1575 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 13 Dec 2024 19:06:10 +0000 Subject: [PATCH 15/15] Use string_view in BoundedReader test --- components/core/tests/test-BoundedReader.cpp | 46 ++++++++++---------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/components/core/tests/test-BoundedReader.cpp b/components/core/tests/test-BoundedReader.cpp index de489b655..9d1a9d2c0 100644 --- a/components/core/tests/test-BoundedReader.cpp +++ b/components/core/tests/test-BoundedReader.cpp @@ -1,6 +1,7 @@ #include #include #include +#include #include @@ -9,13 +10,12 @@ #include "../src/clp/StringReader.hpp" TEST_CASE("Test Bounded Reader", "[BoundedReader]") { - constexpr char cTestString[]{"0123456789"}; - constexpr size_t cTestStringLen{std::char_traits::length(cTestString)}; + constexpr std::string_view cTestString{"0123456789"}; SECTION("BoundedReader does not support try_read_to_delimiter") { clp::StringReader string_reader; - string_reader.open(cTestString); - clp::BoundedReader bounded_reader{&string_reader, cTestStringLen}; + string_reader.open(std::string{cTestString}); + clp::BoundedReader bounded_reader{&string_reader, cTestString.size()}; std::string tmp; REQUIRE(clp::ErrorCode_Unsupported == bounded_reader.try_read_to_delimiter('0', false, false, tmp)); @@ -23,24 +23,24 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { SECTION("BoundedReader does not allow reads beyond end of underlying stream.") { clp::StringReader string_reader; - string_reader.open(cTestString); - clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1}; - std::array buf{}; + string_reader.open(std::string{cTestString}); + clp::BoundedReader bounded_reader{&string_reader, cTestString.size() + 1}; + std::array buf{}; size_t num_bytes_read{}; - auto rc = bounded_reader.try_read(buf.data(), cTestStringLen + 1, num_bytes_read); + auto rc = bounded_reader.try_read(buf.data(), cTestString.size() + 1, num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); - REQUIRE(num_bytes_read == cTestStringLen); - REQUIRE(cTestStringLen == string_reader.get_pos()); - REQUIRE(cTestStringLen == bounded_reader.get_pos()); + REQUIRE(num_bytes_read == cTestString.size()); + REQUIRE(cTestString.size() == string_reader.get_pos()); + REQUIRE(cTestString.size() == bounded_reader.get_pos()); } SECTION("BoundedReader does not allow reads beyond checkpoint.") { clp::StringReader string_reader; - string_reader.open(cTestString); + string_reader.open(std::string{cTestString}); clp::BoundedReader bounded_reader{&string_reader, 1}; - std::array buf{}; + std::array buf{}; size_t num_bytes_read{}; - auto rc = bounded_reader.try_read(buf.data(), cTestStringLen, num_bytes_read); + auto rc = bounded_reader.try_read(buf.data(), cTestString.size(), num_bytes_read); REQUIRE(clp::ErrorCode_Success == rc); REQUIRE(1 == num_bytes_read); REQUIRE(1 == string_reader.get_pos()); @@ -54,7 +54,7 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { SECTION("BoundedReader does allow reads before checkpoint.") { clp::StringReader string_reader; - string_reader.open(cTestString); + string_reader.open(std::string{cTestString}); clp::BoundedReader bounded_reader{&string_reader, 1}; char buf{}; size_t num_bytes_read{}; @@ -67,20 +67,20 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { SECTION("BoundedReader does not allow seeks beyond end of underlying stream.") { clp::StringReader string_reader; - string_reader.open(cTestString); - clp::BoundedReader bounded_reader{&string_reader, cTestStringLen + 1}; - auto rc = bounded_reader.try_seek_from_begin(cTestStringLen + 1); + string_reader.open(std::string{cTestString}); + clp::BoundedReader bounded_reader{&string_reader, cTestString.size() + 1}; + auto rc = bounded_reader.try_seek_from_begin(cTestString.size() + 1); REQUIRE(clp::ErrorCode_EndOfFile == rc); - REQUIRE(cTestStringLen == string_reader.get_pos()); - REQUIRE(cTestStringLen == bounded_reader.get_pos()); + REQUIRE(cTestString.size() == string_reader.get_pos()); + REQUIRE(cTestString.size() == bounded_reader.get_pos()); } SECTION("BoundedReader does not allow seeks beyond checkpoint.") { clp::StringReader string_reader; - string_reader.open(cTestString); + string_reader.open(std::string{cTestString}); clp::BoundedReader bounded_reader{&string_reader, 1}; size_t num_bytes_read{}; - auto rc = bounded_reader.try_seek_from_begin(cTestStringLen); + auto rc = bounded_reader.try_seek_from_begin(cTestString.size()); REQUIRE(clp::ErrorCode_EndOfFile == rc); REQUIRE(1 == string_reader.get_pos()); REQUIRE(1 == bounded_reader.get_pos()); @@ -88,7 +88,7 @@ TEST_CASE("Test Bounded Reader", "[BoundedReader]") { SECTION("BoundedReader does allow seeks before checkpoint.") { clp::StringReader string_reader; - string_reader.open(cTestString); + string_reader.open(std::string{cTestString}); clp::BoundedReader bounded_reader{&string_reader, 2}; size_t num_bytes_read{}; auto rc = bounded_reader.try_seek_from_begin(1);