From 89b0ab2d22db510e799e6f9619e8c8e0678f13ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20Lam?= Date: Mon, 30 Dec 2019 10:48:11 +0100 Subject: [PATCH] StringUtil: Add IsPrintableCharacter and use it Add a function that safely returns whether a character is printable i.e. whether 0x20 <= c <= 0x7e is true. This is done in several places in our codebase and it's easy to run into undefined behaviour if the C version defined in is used instead of this one, since its behaviour is undefined if the character is not representable as an unsigned char. This fixes MemoryViewWidget. --- Source/Core/Common/StringUtil.cpp | 2 +- Source/Core/Common/StringUtil.h | 8 ++++++++ Source/Core/Core/Core.cpp | 10 ++++------ Source/Core/Core/IOS/ES/Formats.cpp | 12 ++---------- Source/Core/Core/IOS/USB/USBV4.cpp | 8 +++----- Source/Core/DiscIO/VolumeWad.cpp | 4 +--- Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp | 5 +++-- 7 files changed, 22 insertions(+), 27 deletions(-) diff --git a/Source/Core/Common/StringUtil.cpp b/Source/Core/Common/StringUtil.cpp index 2369208f45..ac7e32efcf 100644 --- a/Source/Core/Common/StringUtil.cpp +++ b/Source/Core/Common/StringUtil.cpp @@ -71,7 +71,7 @@ std::string HexDump(const u8* data, size_t size) if (row_start + i < size) { char c = static_cast(data[row_start + i]); - out += std::isprint(c, std::locale::classic()) ? c : '.'; + out += IsPrintableCharacter(c) ? c : '.'; } } out += "\n"; diff --git a/Source/Core/Common/StringUtil.h b/Source/Core/Common/StringUtil.h index c47fd1f620..93199917b9 100644 --- a/Source/Core/Common/StringUtil.h +++ b/Source/Core/Common/StringUtil.h @@ -222,3 +222,11 @@ std::string ThousandSeparate(I value, int spaces = 0) return stream.str(); #endif } + +/// Returns whether a character is printable, i.e. whether 0x20 <= c <= 0x7e is true. +/// Use this instead of calling std::isprint directly to ensure +/// the C locale is being used and to avoid possibly undefined behaviour. +inline bool IsPrintableCharacter(char c) +{ + return std::isprint(c, std::locale::classic()); +} diff --git a/Source/Core/Core/Core.cpp b/Source/Core/Core/Core.cpp index 83b3cfa794..131517641b 100644 --- a/Source/Core/Core/Core.cpp +++ b/Source/Core/Core/Core.cpp @@ -4,9 +4,9 @@ #include "Core/Core.h" +#include #include #include -#include #include #include #include @@ -31,6 +31,7 @@ #include "Common/MemoryUtil.h" #include "Common/MsgHandler.h" #include "Common/ScopeGuard.h" +#include "Common/StringUtil.h" #include "Common/Thread.h" #include "Common/Timer.h" #include "Common/Version.h" @@ -160,11 +161,8 @@ void DisplayMessage(std::string message, int time_in_ms) return; // Actually displaying non-ASCII could cause things to go pear-shaped - for (const char& c : message) - { - if (!std::isprint(c, std::locale::classic())) - return; - } + if (!std::all_of(message.begin(), message.end(), IsPrintableCharacter)) + return; Host_UpdateTitle(message); OSD::AddMessage(std::move(message), time_in_ms); diff --git a/Source/Core/Core/IOS/ES/Formats.cpp b/Source/Core/Core/IOS/ES/Formats.cpp index f264d5e494..8e1bea3f24 100644 --- a/Source/Core/Core/IOS/ES/Formats.cpp +++ b/Source/Core/Core/IOS/ES/Formats.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -314,11 +313,7 @@ std::string TMDReader::GetGameID() const std::memcpy(game_id, m_bytes.data() + offsetof(TMDHeader, title_id) + 4, 4); std::memcpy(game_id + 4, m_bytes.data() + offsetof(TMDHeader, group_id), 2); - const bool all_printable = std::all_of(std::begin(game_id), std::end(game_id), [](char c) { - return std::isprint(c, std::locale::classic()); - }); - - if (all_printable) + if (std::all_of(std::begin(game_id), std::end(game_id), IsPrintableCharacter)) return std::string(game_id, sizeof(game_id)); return fmt::format("{:016x}", GetTitleId()); @@ -329,10 +324,7 @@ std::string TMDReader::GetGameTDBID() const const u8* begin = m_bytes.data() + offsetof(TMDHeader, title_id) + 4; const u8* end = begin + 4; - const bool all_printable = - std::all_of(begin, end, [](char c) { return std::isprint(c, std::locale::classic()); }); - - if (all_printable) + if (std::all_of(begin, end, IsPrintableCharacter)) return std::string(begin, end); return fmt::format("{:016x}", GetTitleId()); diff --git a/Source/Core/Core/IOS/USB/USBV4.cpp b/Source/Core/Core/IOS/USB/USBV4.cpp index 782dc463b4..b3f5bf7253 100644 --- a/Source/Core/Core/IOS/USB/USBV4.cpp +++ b/Source/Core/Core/IOS/USB/USBV4.cpp @@ -5,10 +5,11 @@ #include "Core/IOS/USB/USBV4.h" #include -#include +#include #include #include "Common/CommonTypes.h" +#include "Common/StringUtil.h" #include "Common/Swap.h" #include "Core/HW/Memmap.h" #include "Core/IOS/Device.h" @@ -75,11 +76,8 @@ V4GetUSStringMessage::V4GetUSStringMessage(Kernel& ios, const IOCtlRequest& ioct void V4GetUSStringMessage::OnTransferComplete(s32 return_value) const { - const std::locale& c_locale = std::locale::classic(); std::string message = Memory::GetString(data_address); - std::replace_if( - message.begin(), message.end(), [&c_locale](char c) { return !std::isprint(c, c_locale); }, - '?'); + std::replace_if(message.begin(), message.end(), std::not_fn(IsPrintableCharacter), '?'); Memory::CopyToEmu(data_address, message.c_str(), message.size()); TransferCommand::OnTransferComplete(return_value); } diff --git a/Source/Core/DiscIO/VolumeWad.cpp b/Source/Core/DiscIO/VolumeWad.cpp index e471edcd1d..584ae3b5b7 100644 --- a/Source/Core/DiscIO/VolumeWad.cpp +++ b/Source/Core/DiscIO/VolumeWad.cpp @@ -5,7 +5,6 @@ #include #include #include -#include #include #include #include @@ -261,8 +260,7 @@ std::string VolumeWAD::GetMakerID(const Partition& partition) const return "00"; // Some weird channels use 0x0000 in place of the MakerID, so we need a check here - const std::locale& c_locale = std::locale::classic(); - if (!std::isprint(temp[0], c_locale) || !std::isprint(temp[1], c_locale)) + if (!IsPrintableCharacter(temp[0]) || !IsPrintableCharacter(temp[1])) return "00"; return DecodeString(temp); diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index e33a2a8880..5e3d427e96 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -14,6 +14,7 @@ #include #include +#include "Common/StringUtil.h" #include "Core/Core.h" #include "Core/HW/AddressSpace.h" #include "Core/PowerPC/BreakPoints.h" @@ -169,8 +170,8 @@ void MemoryViewWidget::Update() case Type::ASCII: update_values([&accessors](u32 address) { const char value = accessors->ReadU8(address); - return std::isprint(value) ? QString{QChar::fromLatin1(value)} : - QString{QChar::fromLatin1('.')}; + return IsPrintableCharacter(value) ? QString{QChar::fromLatin1(value)} : + QString{QChar::fromLatin1('.')}; }); break; case Type::U16: