From de8430bb29c218dba3a4b66872240a812ef817f4 Mon Sep 17 00:00:00 2001 From: Chris Thrasher Date: Sun, 28 Apr 2024 21:53:11 -0600 Subject: [PATCH] Use `std::optional` rather than sentinel values --- examples/vulkan/Vulkan.cpp | 10 +-- include/SFML/System/FileInputStream.hpp | 16 ++--- include/SFML/System/InputStream.hpp | 26 +++---- include/SFML/System/MemoryInputStream.hpp | 20 +++--- src/SFML/Audio/InputSoundFile.cpp | 2 +- src/SFML/Audio/SoundFileFactory.cpp | 6 +- src/SFML/Audio/SoundFileReaderFlac.cpp | 31 ++++---- src/SFML/Audio/SoundFileReaderMp3.cpp | 10 +-- src/SFML/Audio/SoundFileReaderOgg.cpp | 17 +++-- src/SFML/Audio/SoundFileReaderWav.cpp | 16 ++--- src/SFML/Graphics/Font.cpp | 12 ++-- src/SFML/Graphics/Image.cpp | 9 +-- src/SFML/Graphics/Shader.cpp | 14 ++-- src/SFML/System/Android/ResourceStream.cpp | 51 ++++--------- src/SFML/System/Android/ResourceStream.hpp | 16 ++--- src/SFML/System/FileInputStream.cpp | 39 +++++----- src/SFML/System/MemoryInputStream.cpp | 24 +++---- test/System/FileInputStream.test.cpp | 8 +-- test/System/MemoryInputStream.test.cpp | 83 +++++++++++++++++----- 19 files changed, 219 insertions(+), 191 deletions(-) diff --git a/examples/vulkan/Vulkan.cpp b/examples/vulkan/Vulkan.cpp index a122064f6..07d0830df 100644 --- a/examples/vulkan/Vulkan.cpp +++ b/examples/vulkan/Vulkan.cpp @@ -931,9 +931,10 @@ public: return; } - std::vector buffer(static_cast(file.getSize()) / sizeof(std::uint32_t)); + const auto fileSize = file.getSize().value(); + std::vector buffer(fileSize / sizeof(std::uint32_t)); - if (file.read(buffer.data(), file.getSize()) != file.getSize()) + if (file.read(buffer.data(), fileSize) != file.getSize()) { vulkanAvailable = false; return; @@ -959,9 +960,10 @@ public: return; } - std::vector buffer(static_cast(file.getSize()) / sizeof(std::uint32_t)); + const auto fileSize = file.getSize().value(); + std::vector buffer(fileSize / sizeof(std::uint32_t)); - if (file.read(buffer.data(), file.getSize()) != file.getSize()) + if (file.read(buffer.data(), fileSize) != file.getSize()) { vulkanAvailable = false; return; diff --git a/include/SFML/System/FileInputStream.hpp b/include/SFML/System/FileInputStream.hpp index edbb83ed9..add0c70f6 100644 --- a/include/SFML/System/FileInputStream.hpp +++ b/include/SFML/System/FileInputStream.hpp @@ -111,36 +111,36 @@ public: /// \param data Buffer where to copy the read data /// \param size Desired number of bytes to read /// - /// \return The number of bytes actually read, or -1 on error + /// \return The number of bytes actually read, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - [[nodiscard]] std::int64_t read(void* data, std::int64_t size) override; + [[nodiscard]] std::optional read(void* data, std::size_t size) override; //////////////////////////////////////////////////////////// /// \brief Change the current reading position /// /// \param position The position to seek to, from the beginning /// - /// \return The position actually sought to, or -1 on error + /// \return The position actually sought to, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - [[nodiscard]] std::int64_t seek(std::int64_t position) override; + [[nodiscard]] std::optional seek(std::size_t position) override; //////////////////////////////////////////////////////////// /// \brief Get the current reading position in the stream /// - /// \return The current position, or -1 on error. + /// \return The current position, or `std::nullopt` on error. /// //////////////////////////////////////////////////////////// - [[nodiscard]] std::int64_t tell() override; + [[nodiscard]] std::optional tell() override; //////////////////////////////////////////////////////////// /// \brief Return the size of the stream /// - /// \return The total number of bytes available in the stream, or -1 on error + /// \return The total number of bytes available in the stream, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - std::int64_t getSize() override; + std::optional getSize() override; private: //////////////////////////////////////////////////////////// diff --git a/include/SFML/System/InputStream.hpp b/include/SFML/System/InputStream.hpp index d910cb222..9c46f5c28 100644 --- a/include/SFML/System/InputStream.hpp +++ b/include/SFML/System/InputStream.hpp @@ -31,6 +31,8 @@ #include +#include + #include @@ -58,36 +60,36 @@ public: /// \param data Buffer where to copy the read data /// \param size Desired number of bytes to read /// - /// \return The number of bytes actually read, or -1 on error + /// \return The number of bytes actually read, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - [[nodiscard]] virtual std::int64_t read(void* data, std::int64_t size) = 0; + [[nodiscard]] virtual std::optional read(void* data, std::size_t size) = 0; //////////////////////////////////////////////////////////// /// \brief Change the current reading position /// /// \param position The position to seek to, from the beginning /// - /// \return The position actually sought to, or -1 on error + /// \return The position actually sought to, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - [[nodiscard]] virtual std::int64_t seek(std::int64_t position) = 0; + [[nodiscard]] virtual std::optional seek(std::size_t position) = 0; //////////////////////////////////////////////////////////// /// \brief Get the current reading position in the stream /// - /// \return The current position, or -1 on error. + /// \return The current position, or `std::nullopt` on error. /// //////////////////////////////////////////////////////////// - [[nodiscard]] virtual std::int64_t tell() = 0; + [[nodiscard]] virtual std::optional tell() = 0; //////////////////////////////////////////////////////////// /// \brief Return the size of the stream /// - /// \return The total number of bytes available in the stream, or -1 on error + /// \return The total number of bytes available in the stream, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - virtual std::int64_t getSize() = 0; + virtual std::optional getSize() = 0; }; } // namespace sf @@ -119,13 +121,13 @@ public: /// /// [[nodiscard]] bool open(const std::filesystem::path& filename); /// -/// [[nodiscard]] std::int64_t read(void* data, std::int64_t size); +/// [[nodiscard]] std::optional read(void* data, std::size_t size); /// -/// [[nodiscard]] std::int64_t seek(std::int64_t position); +/// [[nodiscard]] std::optional seek(std::size_t position); /// -/// [[nodiscard]] std::int64_t tell(); +/// [[nodiscard]] std::optional tell(); /// -/// std::int64_t getSize(); +/// std::optional getSize(); /// /// private: /// diff --git a/include/SFML/System/MemoryInputStream.hpp b/include/SFML/System/MemoryInputStream.hpp index 611acf265..18e2b3ef3 100644 --- a/include/SFML/System/MemoryInputStream.hpp +++ b/include/SFML/System/MemoryInputStream.hpp @@ -64,44 +64,44 @@ public: /// \param data Buffer where to copy the read data /// \param size Desired number of bytes to read /// - /// \return The number of bytes actually read, or -1 on error + /// \return The number of bytes actually read, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - [[nodiscard]] std::int64_t read(void* data, std::int64_t size) override; + [[nodiscard]] std::optional read(void* data, std::size_t size) override; //////////////////////////////////////////////////////////// /// \brief Change the current reading position /// /// \param position The position to seek to, from the beginning /// - /// \return The position actually sought to, or -1 on error + /// \return The position actually sought to, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - [[nodiscard]] std::int64_t seek(std::int64_t position) override; + [[nodiscard]] std::optional seek(std::size_t position) override; //////////////////////////////////////////////////////////// /// \brief Get the current reading position in the stream /// - /// \return The current position, or -1 on error. + /// \return The current position, or `std::nullopt` on error. /// //////////////////////////////////////////////////////////// - [[nodiscard]] std::int64_t tell() override; + [[nodiscard]] std::optional tell() override; //////////////////////////////////////////////////////////// /// \brief Return the size of the stream /// - /// \return The total number of bytes available in the stream, or -1 on error + /// \return The total number of bytes available in the stream, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - std::int64_t getSize() override; + std::optional getSize() override; private: //////////////////////////////////////////////////////////// // Member data //////////////////////////////////////////////////////////// const std::byte* m_data{}; //!< Pointer to the data in memory - std::int64_t m_size{}; //!< Total size of the data - std::int64_t m_offset{}; //!< Current reading position + std::size_t m_size{}; //!< Total size of the data + std::size_t m_offset{}; //!< Current reading position }; } // namespace sf diff --git a/src/SFML/Audio/InputSoundFile.cpp b/src/SFML/Audio/InputSoundFile.cpp index 8d5aa44e4..8cef74d9f 100644 --- a/src/SFML/Audio/InputSoundFile.cpp +++ b/src/SFML/Audio/InputSoundFile.cpp @@ -213,7 +213,7 @@ void InputSoundFile::seek(std::uint64_t sampleOffset) //////////////////////////////////////////////////////////// void InputSoundFile::seek(Time timeOffset) { - seek(static_cast(timeOffset.asSeconds() * static_cast(m_sampleRate)) * m_channelMap.size()); + seek(static_cast(timeOffset.asSeconds() * static_cast(m_sampleRate)) * m_channelMap.size()); } diff --git a/src/SFML/Audio/SoundFileFactory.cpp b/src/SFML/Audio/SoundFileFactory.cpp index b8ef7ac41..4ad007e24 100644 --- a/src/SFML/Audio/SoundFileFactory.cpp +++ b/src/SFML/Audio/SoundFileFactory.cpp @@ -58,7 +58,7 @@ std::unique_ptr SoundFileFactory::createReaderFromFilename(cons // Test the filename in all the registered factories for (const auto& [fpCreate, fpCheck] : getReaderFactoryMap()) { - if (stream.seek(0) == -1) + if (!stream.seek(0).has_value()) { err() << "Failed to seek sound stream" << std::endl; return nullptr; @@ -84,7 +84,7 @@ std::unique_ptr SoundFileFactory::createReaderFromMemory(const // Test the stream for all the registered factories for (const auto& [fpCreate, fpCheck] : getReaderFactoryMap()) { - if (stream.seek(0) == -1) + if (!stream.seek(0).has_value()) { err() << "Failed to seek sound stream" << std::endl; return nullptr; @@ -106,7 +106,7 @@ std::unique_ptr SoundFileFactory::createReaderFromStream(InputS // Test the stream for all the registered factories for (const auto& [fpCreate, fpCheck] : getReaderFactoryMap()) { - if (stream.seek(0) == -1) + if (!stream.seek(0).has_value()) { err() << "Failed to seek sound stream" << std::endl; return nullptr; diff --git a/src/SFML/Audio/SoundFileReaderFlac.cpp b/src/SFML/Audio/SoundFileReaderFlac.cpp index 1aa6db862..610084933 100644 --- a/src/SFML/Audio/SoundFileReaderFlac.cpp +++ b/src/SFML/Audio/SoundFileReaderFlac.cpp @@ -44,15 +44,17 @@ FLAC__StreamDecoderReadStatus streamRead(const FLAC__StreamDecoder*, FLAC__byte { auto* data = static_cast(clientData); - const std::int64_t count = data->stream->read(buffer, static_cast(*bytes)); - if (count > 0) + if (const std::optional count = data->stream->read(buffer, *bytes)) { - *bytes = static_cast(count); - return FLAC__STREAM_DECODER_READ_STATUS_CONTINUE; - } - else if (count == 0) - { - return FLAC__STREAM_DECODER_READ_STATUS_END_OF_STREAM; + if (*count > 0) + { + *bytes = *count; + return FLAC__STREAM_DECODER_READ_STATUS_CONTINUE; + } + else + { + return FLAC__STREAM_DECODER_READ_STATUS_END_OF_STREAM; + } } else { @@ -64,8 +66,7 @@ FLAC__StreamDecoderSeekStatus streamSeek(const FLAC__StreamDecoder*, FLAC__uint6 { auto* data = static_cast(clientData); - const std::int64_t position = data->stream->seek(static_cast(absoluteByteOffset)); - if (position >= 0) + if (data->stream->seek(static_cast(absoluteByteOffset)).has_value()) return FLAC__STREAM_DECODER_SEEK_STATUS_OK; else return FLAC__STREAM_DECODER_SEEK_STATUS_ERROR; @@ -75,10 +76,9 @@ FLAC__StreamDecoderTellStatus streamTell(const FLAC__StreamDecoder*, FLAC__uint6 { auto* data = static_cast(clientData); - const std::int64_t position = data->stream->tell(); - if (position >= 0) + if (const std::optional position = data->stream->tell()) { - *absoluteByteOffset = static_cast(position); + *absoluteByteOffset = *position; return FLAC__STREAM_DECODER_TELL_STATUS_OK; } else @@ -91,10 +91,9 @@ FLAC__StreamDecoderLengthStatus streamLength(const FLAC__StreamDecoder*, FLAC__u { auto* data = static_cast(clientData); - const std::int64_t count = data->stream->getSize(); - if (count >= 0) + if (const std::optional count = data->stream->getSize()) { - *streamLength = static_cast(count); + *streamLength = *count; return FLAC__STREAM_DECODER_LENGTH_STATUS_OK; } else diff --git a/src/SFML/Audio/SoundFileReaderMp3.cpp b/src/SFML/Audio/SoundFileReaderMp3.cpp index bf73b66d8..858f45a8a 100644 --- a/src/SFML/Audio/SoundFileReaderMp3.cpp +++ b/src/SFML/Audio/SoundFileReaderMp3.cpp @@ -68,14 +68,14 @@ namespace std::size_t readCallback(void* ptr, std::size_t size, void* data) { auto* stream = static_cast(data); - return static_cast(stream->read(ptr, static_cast(size))); + return stream->read(ptr, size).value_or(-1); } int seekCallback(std::uint64_t offset, void* data) { - auto* stream = static_cast(data); - const std::int64_t position = stream->seek(static_cast(offset)); - return position < 0 ? -1 : 0; + auto* stream = static_cast(data); + const std::optional position = stream->seek(static_cast(offset)); + return position ? 0 : -1; } bool hasValidId3Tag(const std::uint8_t* header) @@ -92,7 +92,7 @@ bool SoundFileReaderMp3::check(InputStream& stream) { std::uint8_t header[10]; - if (static_cast(stream.read(header, static_cast(sizeof(header)))) < sizeof(header)) + if (stream.read(header, sizeof(header)) != sizeof(header)) return false; if (hasValidId3Tag(header)) diff --git a/src/SFML/Audio/SoundFileReaderOgg.cpp b/src/SFML/Audio/SoundFileReaderOgg.cpp index 2b91308c9..ee289e95e 100644 --- a/src/SFML/Audio/SoundFileReaderOgg.cpp +++ b/src/SFML/Audio/SoundFileReaderOgg.cpp @@ -41,29 +41,32 @@ namespace std::size_t read(void* ptr, std::size_t size, std::size_t nmemb, void* data) { auto* stream = static_cast(data); - return static_cast(stream->read(ptr, static_cast(size * nmemb))); + return stream->read(ptr, size * nmemb).value_or(-1); } -int seek(void* data, ogg_int64_t offset, int whence) +int seek(void* data, ogg_int64_t signedOffset, int whence) { auto* stream = static_cast(data); + auto offset = static_cast(signedOffset); switch (whence) { case SEEK_SET: break; case SEEK_CUR: - offset += stream->tell(); + offset += stream->tell().value(); break; case SEEK_END: - offset = stream->getSize() - offset; + offset = stream->getSize().value() - offset; } - return static_cast(stream->seek(offset)); + const std::optional position = stream->seek(offset); + return position ? static_cast(*position) : -1; } long tell(void* data) { - auto* stream = static_cast(data); - return static_cast(stream->tell()); + auto* stream = static_cast(data); + const std::optional position = stream->tell(); + return position ? static_cast(*position) : -1; } ov_callbacks callbacks = {&read, &seek, nullptr, &tell}; diff --git a/src/SFML/Audio/SoundFileReaderWav.cpp b/src/SFML/Audio/SoundFileReaderWav.cpp index 469109837..a32b6c258 100644 --- a/src/SFML/Audio/SoundFileReaderWav.cpp +++ b/src/SFML/Audio/SoundFileReaderWav.cpp @@ -43,13 +43,13 @@ namespace { ma_result onRead(ma_decoder* decoder, void* buffer, size_t bytesToRead, size_t* bytesRead) { - auto* stream = static_cast(decoder->pUserData); - const auto count = stream->read(buffer, static_cast(bytesToRead)); + auto* stream = static_cast(decoder->pUserData); + const std::optional count = stream->read(buffer, bytesToRead); - if (count < 0) + if (!count.has_value()) return MA_ERROR; - *bytesRead = static_cast(count); + *bytesRead = static_cast(*count); return MA_SUCCESS; } @@ -61,19 +61,17 @@ ma_result onSeek(ma_decoder* decoder, ma_int64 byteOffset, ma_seek_origin origin { case ma_seek_origin_start: { - if (stream->seek(byteOffset) < 0) + if (!stream->seek(static_cast(byteOffset)).has_value()) return MA_ERROR; return MA_SUCCESS; } case ma_seek_origin_current: { - const auto currentPosition = stream->tell(); - - if (currentPosition < 0) + if (!stream->tell().has_value()) return MA_ERROR; - if (stream->seek(stream->tell() + byteOffset) < 0) + if (!stream->seek(stream->tell().value() + static_cast(byteOffset)).has_value()) return MA_ERROR; return MA_SUCCESS; diff --git a/src/SFML/Graphics/Font.cpp b/src/SFML/Graphics/Font.cpp index f5eceff67..f9b69b641 100644 --- a/src/SFML/Graphics/Font.cpp +++ b/src/SFML/Graphics/Font.cpp @@ -55,13 +55,11 @@ namespace // FreeType callbacks that operate on a sf::InputStream unsigned long read(FT_Stream rec, unsigned long offset, unsigned char* buffer, unsigned long count) { - const auto convertedOffset = static_cast(offset); - auto* stream = static_cast(rec->descriptor.pointer); - if (stream->seek(convertedOffset) == convertedOffset) + auto* stream = static_cast(rec->descriptor.pointer); + if (stream->seek(offset) == offset) { if (count > 0) - return static_cast( - stream->read(reinterpret_cast(buffer), static_cast(count))); + return static_cast(stream->read(reinterpret_cast(buffer), count).value()); else return 0; } @@ -244,7 +242,7 @@ std::optional Font::loadFromStream(InputStream& stream) } // Make sure that the stream's reading position is at the beginning - if (stream.seek(0) == -1) + if (!stream.seek(0).has_value()) { err() << "Failed to seek font stream" << std::endl; return std::nullopt; @@ -252,7 +250,7 @@ std::optional Font::loadFromStream(InputStream& stream) // Prepare a wrapper for our stream, that we'll pass to FreeType callbacks fontHandles->streamRec.base = nullptr; - fontHandles->streamRec.size = static_cast(stream.getSize()); + fontHandles->streamRec.size = static_cast(stream.getSize().value()); fontHandles->streamRec.pos = 0; fontHandles->streamRec.descriptor.pointer = &stream; fontHandles->streamRec.read = &read; diff --git a/src/SFML/Graphics/Image.cpp b/src/SFML/Graphics/Image.cpp index e600d255a..be8c18d51 100644 --- a/src/SFML/Graphics/Image.cpp +++ b/src/SFML/Graphics/Image.cpp @@ -55,14 +55,15 @@ namespace // stb_image callbacks that operate on a sf::InputStream int read(void* user, char* data, int size) { - auto& stream = *static_cast(user); - return static_cast(stream.read(data, size)); + auto& stream = *static_cast(user); + const std::optional count = stream.read(data, static_cast(size)); + return count ? static_cast(*count) : -1; } void skip(void* user, int size) { auto& stream = *static_cast(user); - if (stream.seek(stream.tell() + size) == -1) + if (!stream.seek(stream.tell().value() + static_cast(size)).has_value()) sf::err() << "Failed to seek image loader input stream" << std::endl; } @@ -233,7 +234,7 @@ std::optional Image::loadFromMemory(const void* data, std::size_t size) std::optional Image::loadFromStream(InputStream& stream) { // Make sure that the stream's reading position is at the beginning - if (stream.seek(0) == -1) + if (!stream.seek(0).has_value()) { err() << "Failed to seek image stream" << std::endl; return std::nullopt; diff --git a/src/SFML/Graphics/Shader.cpp b/src/SFML/Graphics/Shader.cpp index 09e5d3604..7228c9c53 100644 --- a/src/SFML/Graphics/Shader.cpp +++ b/src/SFML/Graphics/Shader.cpp @@ -104,20 +104,20 @@ bool getFileContents(const std::filesystem::path& filename, std::vector& b // Read the contents of a stream into an array of char bool getStreamContents(sf::InputStream& stream, std::vector& buffer) { - bool success = false; - const std::int64_t size = stream.getSize(); - if (size > 0) + bool success = false; + const std::optional size = stream.getSize(); + if (size > std::size_t{0}) { - buffer.resize(static_cast(size)); + buffer.resize(*size); - if (stream.seek(0) == -1) + if (!stream.seek(0).has_value()) { sf::err() << "Failed to seek shader stream" << std::endl; return false; } - const std::int64_t read = stream.read(buffer.data(), size); - success = (read == size); + const std::optional read = stream.read(buffer.data(), *size); + success = (read == size); } buffer.push_back('\0'); return success; diff --git a/src/SFML/System/Android/ResourceStream.cpp b/src/SFML/System/Android/ResourceStream.cpp index 00e07c280..b2c48f344 100644 --- a/src/SFML/System/Android/ResourceStream.cpp +++ b/src/SFML/System/Android/ResourceStream.cpp @@ -41,62 +41,41 @@ ResourceStream::ResourceStream(const std::filesystem::path& filename) ActivityStates& states = getActivity(); const std::lock_guard lock(states.mutex); m_file.reset(AAssetManager_open(states.activity->assetManager, filename.c_str(), AASSET_MODE_UNKNOWN)); + assert(m_file && "Failed to initialize ResourceStream file"); } //////////////////////////////////////////////////////////// -std::int64_t ResourceStream::read(void* data, std::int64_t size) +std::optional ResourceStream::read(void* data, std::size_t size) { - if (m_file) - { - return AAsset_read(m_file.get(), data, static_cast(size)); - } - else - { - return -1; - } + const auto numBytesRead = AAsset_read(m_file.get(), data, size); + if (numBytesRead < 0) + return std::nullopt; + return numBytesRead; } //////////////////////////////////////////////////////////// -std::int64_t ResourceStream::seek(std::int64_t position) +std::optional ResourceStream::seek(std::size_t position) { - if (m_file) - { - return AAsset_seek(m_file.get(), static_cast(position), SEEK_SET); - } - else - { - return -1; - } + const auto newPosition = AAsset_seek(m_file.get(), static_cast(position), SEEK_SET); + if (newPosition < 0) + return std::nullopt; + return newPosition; } //////////////////////////////////////////////////////////// -std::int64_t ResourceStream::tell() +std::optional ResourceStream::tell() { - if (m_file) - { - return getSize() - AAsset_getRemainingLength(m_file.get()); - } - else - { - return -1; - } + return getSize().value() - static_cast(AAsset_getRemainingLength(m_file.get())); } //////////////////////////////////////////////////////////// -std::int64_t ResourceStream::getSize() +std::optional ResourceStream::getSize() { - if (m_file) - { - return AAsset_getLength(m_file.get()); - } - else - { - return -1; - } + return AAsset_getLength(m_file.get()); } diff --git a/src/SFML/System/Android/ResourceStream.hpp b/src/SFML/System/Android/ResourceStream.hpp index a77f2d77a..70f2552ba 100644 --- a/src/SFML/System/Android/ResourceStream.hpp +++ b/src/SFML/System/Android/ResourceStream.hpp @@ -60,36 +60,36 @@ public: /// \param data Buffer where the asset data is copied /// \param size Number of bytes read /// - /// \return The number of bytes actually read, or -1 on error + /// \return The number of bytes actually read, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - std::int64_t read(void* data, std::int64_t size) override; + std::optional read(void* data, std::size_t size) override; //////////////////////////////////////////////////////////// /// \brief Change the current reading position in the asset file /// /// \param position The position to seek to, from the beginning /// - /// \return The position actually sought to, or -1 on error + /// \return The position actually sought to, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - std::int64_t seek(std::int64_t position) override; + std::optional seek(std::size_t position) override; //////////////////////////////////////////////////////////// /// \brief Get the current reading position in the asset file /// - /// \return The current position, or -1 on error. + /// \return The current position, or `std::nullopt` on error. /// //////////////////////////////////////////////////////////// - std::int64_t tell() override; + std::optional tell() override; //////////////////////////////////////////////////////////// /// \brief Return the size of the asset file /// - /// \return The total number of bytes available in the asset, or -1 on error + /// \return The total number of bytes available in the asset, or `std::nullopt` on error /// //////////////////////////////////////////////////////////// - std::int64_t getSize() override; + std::optional getSize() override; private: //////////////////////////////////////////////////////////// diff --git a/src/SFML/System/FileInputStream.cpp b/src/SFML/System/FileInputStream.cpp index b523ff797..6aef44a62 100644 --- a/src/SFML/System/FileInputStream.cpp +++ b/src/SFML/System/FileInputStream.cpp @@ -79,78 +79,79 @@ bool FileInputStream::open(const std::filesystem::path& filename) //////////////////////////////////////////////////////////// -std::int64_t FileInputStream::read(void* data, std::int64_t size) +std::optional FileInputStream::read(void* data, std::size_t size) { #ifdef SFML_SYSTEM_ANDROID if (priv::getActivityStatesPtr() != nullptr) { if (!m_androidFile) - return -1; + return std::nullopt; return m_androidFile->read(data, size); } #endif if (!m_file) - return -1; - return static_cast(std::fread(data, 1, static_cast(size), m_file.get())); + return std::nullopt; + return std::fread(data, 1, size, m_file.get()); } //////////////////////////////////////////////////////////// -std::int64_t FileInputStream::seek(std::int64_t position) +std::optional FileInputStream::seek(std::size_t position) { #ifdef SFML_SYSTEM_ANDROID if (priv::getActivityStatesPtr() != nullptr) { if (!m_androidFile) - return -1; + return std::nullopt; return m_androidFile->seek(position); } #endif if (!m_file) - return -1; + return std::nullopt; if (std::fseek(m_file.get(), static_cast(position), SEEK_SET)) - return -1; + return std::nullopt; return tell(); } //////////////////////////////////////////////////////////// -std::int64_t FileInputStream::tell() +std::optional FileInputStream::tell() { #ifdef SFML_SYSTEM_ANDROID if (priv::getActivityStatesPtr() != nullptr) { if (!m_androidFile) - return -1; + return std::nullopt; return m_androidFile->tell(); } #endif if (!m_file) - return -1; - return std::ftell(m_file.get()); + return std::nullopt; + const auto position = std::ftell(m_file.get()); + return position < 0 ? std::nullopt : std::optional(position); } //////////////////////////////////////////////////////////// -std::int64_t FileInputStream::getSize() +std::optional FileInputStream::getSize() { #ifdef SFML_SYSTEM_ANDROID if (priv::getActivityStatesPtr() != nullptr) { if (!m_androidFile) - return -1; + return std::nullopt; return m_androidFile->getSize(); } #endif if (!m_file) - return -1; - const std::int64_t position = tell(); + return std::nullopt; + const auto position = tell().value(); std::fseek(m_file.get(), 0, SEEK_END); - const std::int64_t size = tell(); + const std::optional size = tell(); - if (seek(position) == -1) - return -1; + if (!seek(position).has_value()) + return std::nullopt; return size; } diff --git a/src/SFML/System/MemoryInputStream.cpp b/src/SFML/System/MemoryInputStream.cpp index 6a49b1c77..abebd2087 100644 --- a/src/SFML/System/MemoryInputStream.cpp +++ b/src/SFML/System/MemoryInputStream.cpp @@ -27,6 +27,8 @@ //////////////////////////////////////////////////////////// #include +#include + #include @@ -36,20 +38,18 @@ namespace sf void MemoryInputStream::open(const void* data, std::size_t sizeInBytes) { m_data = static_cast(data); - m_size = static_cast(sizeInBytes); + m_size = sizeInBytes; m_offset = 0; } //////////////////////////////////////////////////////////// -std::int64_t MemoryInputStream::read(void* data, std::int64_t size) +std::optional MemoryInputStream::read(void* data, std::size_t size) { if (!m_data) - return -1; - - const std::int64_t endPosition = m_offset + size; - const std::int64_t count = endPosition <= m_size ? size : m_size - m_offset; + return std::nullopt; + const std::size_t count = std::min(size, m_size - m_offset); if (count > 0) { std::memcpy(data, m_data + m_offset, static_cast(count)); @@ -61,10 +61,10 @@ std::int64_t MemoryInputStream::read(void* data, std::int64_t size) //////////////////////////////////////////////////////////// -std::int64_t MemoryInputStream::seek(std::int64_t position) +std::optional MemoryInputStream::seek(std::size_t position) { if (!m_data) - return -1; + return std::nullopt; m_offset = position < m_size ? position : m_size; return m_offset; @@ -72,20 +72,20 @@ std::int64_t MemoryInputStream::seek(std::int64_t position) //////////////////////////////////////////////////////////// -std::int64_t MemoryInputStream::tell() +std::optional MemoryInputStream::tell() { if (!m_data) - return -1; + return std::nullopt; return m_offset; } //////////////////////////////////////////////////////////// -std::int64_t MemoryInputStream::getSize() +std::optional MemoryInputStream::getSize() { if (!m_data) - return -1; + return std::nullopt; return m_size; } diff --git a/test/System/FileInputStream.test.cpp b/test/System/FileInputStream.test.cpp index 43eccb28c..ae5386c52 100644 --- a/test/System/FileInputStream.test.cpp +++ b/test/System/FileInputStream.test.cpp @@ -75,10 +75,10 @@ TEST_CASE("[System] sf::FileInputStream") SECTION("Default constructor") { sf::FileInputStream fileInputStream; - CHECK(fileInputStream.read(nullptr, 0) == -1); - CHECK(fileInputStream.seek(0) == -1); - CHECK(fileInputStream.tell() == -1); - CHECK(fileInputStream.getSize() == -1); + CHECK(fileInputStream.read(nullptr, 0) == std::nullopt); + CHECK(fileInputStream.seek(0) == std::nullopt); + CHECK(fileInputStream.tell() == std::nullopt); + CHECK(fileInputStream.getSize() == std::nullopt); } const TemporaryFile temporaryFile("Hello world"); diff --git a/test/System/MemoryInputStream.test.cpp b/test/System/MemoryInputStream.test.cpp index 5b5845d65..0bfddeebb 100644 --- a/test/System/MemoryInputStream.test.cpp +++ b/test/System/MemoryInputStream.test.cpp @@ -16,28 +16,73 @@ TEST_CASE("[System] sf::MemoryInputStream") STATIC_CHECK(std::is_nothrow_move_assignable_v); } - SECTION("Empty stream") + SECTION("Default constructor") { - sf::MemoryInputStream mis; - - CHECK(mis.read(nullptr, 0) == -1); - CHECK(mis.seek(0) == -1); - CHECK(mis.tell() == -1); - CHECK(mis.getSize() == -1); + sf::MemoryInputStream memoryInputStream; + CHECK(memoryInputStream.read(nullptr, 0) == std::nullopt); + CHECK(memoryInputStream.seek(0) == std::nullopt); + CHECK(memoryInputStream.tell() == std::nullopt); + CHECK(memoryInputStream.getSize() == std::nullopt); } - SECTION("Open memory stream") - { - using namespace std::literals::string_view_literals; - constexpr auto memoryContents = "hello world"sv; - sf::MemoryInputStream mis; - mis.open(memoryContents.data(), sizeof(char) * memoryContents.size()); + using namespace std::literals::string_view_literals; - std::array buffer{}; - CHECK(mis.read(buffer.data(), 5) == 5); - CHECK(std::string_view(buffer.data(), 5) == std::string_view(memoryContents.data(), 5)); - CHECK(mis.seek(10) == 10); - CHECK(mis.tell() == 10); - CHECK(mis.getSize() == 11); + SECTION("open()") + { + sf::MemoryInputStream memoryInputStream; + memoryInputStream.open(nullptr, 0); + CHECK(memoryInputStream.tell() == std::nullopt); + CHECK(memoryInputStream.getSize() == std::nullopt); + + static constexpr auto input = "hello world"sv; + memoryInputStream.open(input.data(), input.size()); + CHECK(memoryInputStream.tell().value() == 0); + CHECK(memoryInputStream.getSize().value() == input.size()); + } + + SECTION("read()") + { + static constexpr auto input = "hello world"sv; + sf::MemoryInputStream memoryInputStream; + memoryInputStream.open(input.data(), input.size()); + CHECK(memoryInputStream.tell().value() == 0); + CHECK(memoryInputStream.getSize().value() == input.size()); + + // Read within input + std::array output{}; + CHECK(memoryInputStream.read(output.data(), 5).value() == 5); + CHECK(std::string_view(output.data(), 5) == "hello"sv); + CHECK(memoryInputStream.tell().value() == 5); + CHECK(memoryInputStream.getSize().value() == input.size()); + + // Read beyond input + CHECK(memoryInputStream.read(output.data(), 100).value() == 6); + CHECK(std::string_view(output.data(), 6) == " world"sv); + CHECK(memoryInputStream.tell().value() == 11); + CHECK(memoryInputStream.getSize().value() == input.size()); + } + + SECTION("seek()") + { + static constexpr auto input = "We Love SFML!"sv; + sf::MemoryInputStream memoryInputStream; + memoryInputStream.open(input.data(), input.size()); + CHECK(memoryInputStream.tell().value() == 0); + CHECK(memoryInputStream.getSize().value() == input.size()); + + SECTION("Seek within input") + { + CHECK(memoryInputStream.seek(0).value() == 0); + CHECK(memoryInputStream.tell().value() == 0); + + CHECK(memoryInputStream.seek(5).value() == 5); + CHECK(memoryInputStream.tell().value() == 5); + } + + SECTION("Seek beyond input") + { + CHECK(memoryInputStream.seek(1'000).value() == input.size()); + CHECK(memoryInputStream.tell().value() == input.size()); + } } }