From a303cee386b6d275cc8354934ef849b8738738d7 Mon Sep 17 00:00:00 2001 From: vittorioromeo Date: Mon, 10 Jun 2024 17:11:43 +0200 Subject: [PATCH] Consistently print errors in factory functions --- include/SFML/Audio/Music.hpp | 3 ++- include/SFML/Network/IpAddress.hpp | 6 ++++-- src/SFML/Audio/InputSoundFile.cpp | 26 ++++++++++++++++++++++++++ src/SFML/Audio/Music.cpp | 11 +++++++---- src/SFML/Audio/OutputSoundFile.cpp | 6 ++++++ src/SFML/Audio/SoundBuffer.cpp | 19 +++++++++++++------ src/SFML/Graphics/Texture.cpp | 30 +++++++++++++++++------------- src/SFML/Network/IpAddress.cpp | 22 ++++++++++++++++++++-- src/SFML/Window/Cursor.cpp | 11 +++++++++++ 9 files changed, 106 insertions(+), 28 deletions(-) diff --git a/include/SFML/Audio/Music.hpp b/include/SFML/Audio/Music.hpp index fa8a2c55b..08742b539 100644 --- a/include/SFML/Audio/Music.hpp +++ b/include/SFML/Audio/Music.hpp @@ -235,7 +235,8 @@ private: /// \brief Try opening the music file from an optional input sound file /// //////////////////////////////////////////////////////////// - [[nodiscard]] static std::optional tryOpenFromFile(std::optional&& optFile); + [[nodiscard]] static std::optional tryOpenFromInputSoundFile(std::optional&& optFile, + const char* errorContext); //////////////////////////////////////////////////////////// /// \brief Initialize the internal state after loading a new music diff --git a/include/SFML/Network/IpAddress.hpp b/include/SFML/Network/IpAddress.hpp index ef9d89961..4ca5d5038 100644 --- a/include/SFML/Network/IpAddress.hpp +++ b/include/SFML/Network/IpAddress.hpp @@ -56,6 +56,8 @@ public: /// /// \param address IP address or network name /// + /// \return Address if provided argument was valid, otherwise `std::nullopt` + /// //////////////////////////////////////////////////////////// [[nodiscard]] static std::optional resolve(std::string_view address); @@ -128,7 +130,7 @@ public: /// Unlike getPublicAddress, this function is fast and may be /// used safely anywhere. /// - /// \return Local IP address of the computer + /// \return Local IP address of the computer on success, `std::nullopt` otherwise /// /// \see getPublicAddress /// @@ -152,7 +154,7 @@ public: /// /// \param timeout Maximum time to wait /// - /// \return Public IP address of the computer + /// \return Public IP address of the computer on success, `std::nullopt` otherwise /// /// \see getLocalAddress /// diff --git a/src/SFML/Audio/InputSoundFile.cpp b/src/SFML/Audio/InputSoundFile.cpp index 843934548..47fd61643 100644 --- a/src/SFML/Audio/InputSoundFile.cpp +++ b/src/SFML/Audio/InputSoundFile.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -71,19 +72,32 @@ std::optional InputSoundFile::openFromFile(const std::filesystem // Find a suitable reader for the file type auto reader = SoundFileFactory::createReaderFromFilename(filename); if (!reader) + { + // Error message generated in called function. return std::nullopt; + } // Wrap the file into a stream auto file = std::make_unique(); // Open it if (!file->open(filename)) + { + err() << "Failed to open input sound file from file (couldn't open file input stream)\n" + << formatDebugPathInfo(filename) << std::endl; + return std::nullopt; + } // Pass the stream to the reader auto info = reader->open(*file); if (!info) + { + err() << "Failed to open input sound file from file (reader open failure)\n" + << formatDebugPathInfo(filename) << std::endl; + return std::nullopt; + } return InputSoundFile(std::move(reader), std::move(file), info->sampleCount, info->sampleRate, std::move(info->channelMap)); } @@ -95,7 +109,10 @@ std::optional InputSoundFile::openFromMemory(const void* data, s // Find a suitable reader for the file type auto reader = SoundFileFactory::createReaderFromMemory(data, sizeInBytes); if (!reader) + { + // Error message generated in called function. return std::nullopt; + } // Wrap the memory file into a stream auto memory = std::make_unique(data, sizeInBytes); @@ -103,7 +120,10 @@ std::optional InputSoundFile::openFromMemory(const void* data, s // Pass the stream to the reader auto info = reader->open(*memory); if (!info) + { + err() << "Failed to open input sound file from memory (reader open failure)" << std::endl; return std::nullopt; + } return InputSoundFile(std::move(reader), std::move(memory), info->sampleCount, info->sampleRate, std::move(info->channelMap)); } @@ -115,7 +135,10 @@ std::optional InputSoundFile::openFromStream(InputStream& stream // Find a suitable reader for the file type auto reader = SoundFileFactory::createReaderFromStream(stream); if (!reader) + { + // Error message generated in called function. return std::nullopt; + } // Don't forget to reset the stream to its beginning before re-opening it if (stream.seek(0) != 0) @@ -127,7 +150,10 @@ std::optional InputSoundFile::openFromStream(InputStream& stream // Pass the stream to the reader auto info = reader->open(stream); if (!info) + { + err() << "Failed to open input sound file from stream (reader open failure)" << std::endl; return std::nullopt; + } return InputSoundFile(std::move(reader), {&stream, false}, info->sampleCount, info->sampleRate, std::move(info->channelMap)); } diff --git a/src/SFML/Audio/Music.cpp b/src/SFML/Audio/Music.cpp index fc16d55b3..03afb7ea0 100644 --- a/src/SFML/Audio/Music.cpp +++ b/src/SFML/Audio/Music.cpp @@ -79,10 +79,13 @@ Music& Music::operator=(Music&&) noexcept = default; //////////////////////////////////////////////////////////// -std::optional Music::tryOpenFromFile(std::optional&& optFile) +std::optional Music::tryOpenFromInputSoundFile(std::optional&& optFile, const char* errorContext) { if (!optFile.has_value()) + { + err() << "Failed to open music from " << errorContext << std::endl; return std::nullopt; + } // TODO: apply RVO here via passkey idiom return Music(std::move(*optFile)); @@ -92,21 +95,21 @@ std::optional Music::tryOpenFromFile(std::optional&& optF //////////////////////////////////////////////////////////// std::optional Music::openFromFile(const std::filesystem::path& filename) { - return tryOpenFromFile(InputSoundFile::openFromFile(filename)); + return tryOpenFromInputSoundFile(InputSoundFile::openFromFile(filename), "file"); } //////////////////////////////////////////////////////////// std::optional Music::openFromMemory(const void* data, std::size_t sizeInBytes) { - return tryOpenFromFile(InputSoundFile::openFromMemory(data, sizeInBytes)); + return tryOpenFromInputSoundFile(InputSoundFile::openFromMemory(data, sizeInBytes), "memory"); } //////////////////////////////////////////////////////////// std::optional Music::openFromStream(InputStream& stream) { - return tryOpenFromFile(InputSoundFile::openFromStream(stream)); + return tryOpenFromInputSoundFile(InputSoundFile::openFromStream(stream), "stream"); } diff --git a/src/SFML/Audio/OutputSoundFile.cpp b/src/SFML/Audio/OutputSoundFile.cpp index 4ab0c81cb..77a264a4a 100644 --- a/src/SFML/Audio/OutputSoundFile.cpp +++ b/src/SFML/Audio/OutputSoundFile.cpp @@ -29,6 +29,8 @@ #include #include +#include + #include @@ -44,11 +46,15 @@ std::optional OutputSoundFile::openFromFile( // Find a suitable writer for the file type auto writer = SoundFileFactory::createWriterFromFilename(filename); if (!writer) + { + // Error message generated in called function. return std::nullopt; + } // Pass the stream to the reader if (!writer->open(filename, sampleRate, channelCount, channelMap)) { + err() << "Failed to open output sound file from file (writer open failure)" << std::endl; return std::nullopt; } diff --git a/src/SFML/Audio/SoundBuffer.cpp b/src/SFML/Audio/SoundBuffer.cpp index 8431e2d5b..ae9b331b0 100644 --- a/src/SFML/Audio/SoundBuffer.cpp +++ b/src/SFML/Audio/SoundBuffer.cpp @@ -72,8 +72,9 @@ std::optional SoundBuffer::loadFromFile(const std::filesystem::path { if (auto file = InputSoundFile::openFromFile(filename)) return initialize(*file); - else - return std::nullopt; + + err() << "Failed to open sound buffer from file" << std::endl; + return std::nullopt; } @@ -82,8 +83,9 @@ std::optional SoundBuffer::loadFromMemory(const void* data, std::si { if (auto file = InputSoundFile::openFromMemory(data, sizeInBytes)) return initialize(*file); - else - return std::nullopt; + + err() << "Failed to open sound buffer from memory" << std::endl; + return std::nullopt; } @@ -92,8 +94,9 @@ std::optional SoundBuffer::loadFromStream(InputStream& stream) { if (auto file = InputSoundFile::openFromStream(stream)) return initialize(*file); - else - return std::nullopt; + + err() << "Failed to open sound buffer from stream" << std::endl; + return std::nullopt; } @@ -223,7 +226,11 @@ std::optional SoundBuffer::initialize(InputSoundFile& file) // Update the internal buffer with the new samples SoundBuffer soundBuffer(std::move(samples)); if (!soundBuffer.update(file.getChannelCount(), file.getSampleRate(), file.getChannelMap())) + { + err() << "Failed to initialize sound buffer (internal update failure)" << std::endl; return std::nullopt; + } + return soundBuffer; } else diff --git a/src/SFML/Graphics/Texture.cpp b/src/SFML/Graphics/Texture.cpp index 76644011e..2ddaab46e 100644 --- a/src/SFML/Graphics/Texture.cpp +++ b/src/SFML/Graphics/Texture.cpp @@ -266,30 +266,33 @@ std::optional Texture::create(const Vector2u& size, bool sRgb) //////////////////////////////////////////////////////////// std::optional Texture::loadFromFile(const std::filesystem::path& filename, bool sRgb, const IntRect& area) { - const auto image = sf::Image::loadFromFile(filename); - if (!image) - return std::nullopt; - return loadFromImage(*image, sRgb, area); + if (const auto image = sf::Image::loadFromFile(filename)) + return loadFromImage(*image, sRgb, area); + + err() << "Failed to load texture from file" << std::endl; + return std::nullopt; } //////////////////////////////////////////////////////////// std::optional Texture::loadFromMemory(const void* data, std::size_t size, bool sRgb, const IntRect& area) { - const auto image = sf::Image::loadFromMemory(data, size); - if (!image) - return std::nullopt; - return loadFromImage(*image, sRgb, area); + if (const auto image = sf::Image::loadFromMemory(data, size)) + return loadFromImage(*image, sRgb, area); + + err() << "Failed to load texture from memory" << std::endl; + return std::nullopt; } //////////////////////////////////////////////////////////// std::optional Texture::loadFromStream(InputStream& stream, bool sRgb, const IntRect& area) { - const auto image = sf::Image::loadFromStream(stream); - if (!image) - return std::nullopt; - return loadFromImage(*image, sRgb, area); + if (const auto image = sf::Image::loadFromStream(stream)) + return loadFromImage(*image, sRgb, area); + + err() << "Failed to load texture from stream" << std::endl; + return std::nullopt; } @@ -307,11 +310,11 @@ std::optional Texture::loadFromImage(const Image& image, bool sRgb, con if (auto texture = sf::Texture::create(image.getSize(), sRgb)) { texture->update(image); - return texture; } else { + // Error message generated in called function. return std::nullopt; } } @@ -354,6 +357,7 @@ std::optional Texture::loadFromImage(const Image& image, bool sRgb, con } else { + // Error message generated in called function. return std::nullopt; } } diff --git a/src/SFML/Network/IpAddress.cpp b/src/SFML/Network/IpAddress.cpp index 2abb4abeb..d874e402d 100644 --- a/src/SFML/Network/IpAddress.cpp +++ b/src/SFML/Network/IpAddress.cpp @@ -29,6 +29,8 @@ #include #include +#include + #include #include @@ -49,7 +51,10 @@ std::optional IpAddress::resolve(std::string_view address) using namespace std::string_view_literals; if (address.empty()) + { + // Not generating en error message here as resolution failure is a valid outcome. return std::nullopt; + } if (address == "255.255.255.255"sv) { @@ -81,6 +86,7 @@ std::optional IpAddress::resolve(std::string_view address) return IpAddress(ntohl(ip)); } + // Not generating en error message here as resolution failure is a valid outcome. return std::nullopt; } @@ -125,13 +131,18 @@ std::optional IpAddress::getLocalAddress() // Create the socket const SocketHandle sock = socket(PF_INET, SOCK_DGRAM, 0); if (sock == priv::SocketImpl::invalidSocket()) + { + err() << "Failed to retrieve local address (invalid socket)" << std::endl; return std::nullopt; + } // Connect the socket to localhost on any port sockaddr_in address = priv::SocketImpl::createAddress(ntohl(INADDR_LOOPBACK), 9); if (connect(sock, reinterpret_cast(&address), sizeof(address)) == -1) { priv::SocketImpl::close(sock); + + err() << "Failed to retrieve local address (socket connection failure)" << std::endl; return std::nullopt; } @@ -140,6 +151,8 @@ std::optional IpAddress::getLocalAddress() if (getsockname(sock, reinterpret_cast(&address), &size) == -1) { priv::SocketImpl::close(sock); + + err() << "Failed to retrieve local address (socket local address retrieval failure)" << std::endl; return std::nullopt; } @@ -163,10 +176,15 @@ std::optional IpAddress::getPublicAddress(Time timeout) Http server("www.sfml-dev.org"); const Http::Request request("/ip-provider.php", Http::Request::Method::Get); const Http::Response page = server.sendRequest(request, timeout); - if (page.getStatus() == Http::Response::Status::Ok) + + const Http::Response::Status status = page.getStatus(); + + if (status == Http::Response::Status::Ok) return IpAddress::resolve(page.getBody()); - // Something failed: return an invalid address + err() << "Failed to retrieve public address from external IP resolution server (HTTP response status " + << static_cast(status) << ")" << std::endl; + return std::nullopt; } diff --git a/src/SFML/Window/Cursor.cpp b/src/SFML/Window/Cursor.cpp index d5a02810e..e3769fa6b 100644 --- a/src/SFML/Window/Cursor.cpp +++ b/src/SFML/Window/Cursor.cpp @@ -28,9 +28,11 @@ #include #include +#include #include #include +#include namespace sf { @@ -57,11 +59,17 @@ Cursor& Cursor::operator=(Cursor&&) noexcept = default; std::optional Cursor::loadFromPixels(const std::uint8_t* pixels, Vector2u size, Vector2u hotspot) { if ((pixels == nullptr) || (size.x == 0) || (size.y == 0)) + { + err() << "Failed to load cursor from pixels (invalid arguments)" << std::endl; return std::nullopt; + } Cursor cursor; if (!cursor.m_impl->loadFromPixels(pixels, size, hotspot)) + { + // Error message generated in called function. return std::nullopt; + } return cursor; } @@ -72,7 +80,10 @@ std::optional Cursor::loadFromSystem(Type type) { Cursor cursor; if (!cursor.m_impl->loadFromSystem(type)) + { + // Error message generated in called function. return std::nullopt; + } return cursor; }