Skip to content
Permalink
Browse files
Merge pull request #9260 from leoetlino/fmt-checks 
Common/Log: Add compile-time format string checks
  • Loading branch information
@leoetlino
leoetlino committed Nov 20, 2020
2 parents 1d46724 + dde6090 commit 419dfe4
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 22 deletions.
1 Source/Core/Common/CMakeLists.txt
View file
@@ -53,6 +53,7 @@ add_library(common
Flag.h
FloatUtils.cpp
FloatUtils.h
FormatUtil.h
FPURoundMode.h
GekkoDisassembler.cpp
GekkoDisassembler.h
3 Source/Core/Common/Common.vcxproj
View file
@@ -58,6 +58,7 @@
<ClInclude Include="FileUtil.h" />
<ClInclude Include="FixedSizeQueue.h" />
<ClInclude Include="Flag.h" />
<ClInclude Include="FormatUtil.h" />
<ClInclude Include="FPURoundMode.h" />
<ClInclude Include="GekkoDisassembler.h" />
<ClInclude Include="GL\GLExtensions\AMD_pinned_memory.h" />
@@ -283,4 +284,4 @@
<Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
<ImportGroup Label="ExtensionTargets">
</ImportGroup>
</Project>
</Project>
3 Source/Core/Common/Common.vcxproj.filters
View file
@@ -48,6 +48,7 @@
<ClInclude Include="FixedSizeQueue.h" />
<ClInclude Include="Flag.h" />
<ClInclude Include="FloatUtils.h" />
<ClInclude Include="FormatUtil.h" />
<ClInclude Include="FPURoundMode.h" />
<ClInclude Include="Hash.h" />
<ClInclude Include="HttpRequest.h" />
@@ -376,4 +377,4 @@
<ItemGroup>
<Natvis Include="BitField.natvis" />
</ItemGroup>
</Project>
</Project>
41 Source/Core/Common/FormatUtil.h
View file
@@ -0,0 +1,41 @@
// Copyright 2020 Dolphin Emulator Project
// Licensed under GPLv2+
// Refer to the license.txt file included.

#pragma once

#include <cstddef>
#include <string_view>

namespace Common
{
constexpr std::size_t CountFmtReplacementFields(std::string_view s)
{
std::size_t count = 0;
for (std::size_t i = 0; i < s.size(); ++i)
{
if (s[i] != '{')
continue;

// If the opening brace is followed by another brace, what we have is
// an escaped brace, not a replacement field.
if (i + 1 < s.size() && s[i + 1] == '{')
{
// Skip the second brace.
// This ensures that e.g. {{{}}} is counted correctly: when the first brace character
// is read and detected as being part of an '{{' escape sequence, the second character
// is skipped so the most inner brace (the third character) is not detected
// as the end of an '{{' pair.
++i;
continue;
}

++count;
}
return count;
}

static_assert(CountFmtReplacementFields("") == 0);
static_assert(CountFmtReplacementFields("{} test {:x}") == 2);
static_assert(CountFmtReplacementFields("{} {{}} test {{{}}}") == 2);
} // namespace Common
25 Source/Core/Common/Logging/Log.h
View file
@@ -4,8 +4,10 @@

#pragma once

#include <cstddef>
#include <fmt/format.h>
#include <string_view>
#include "Common/FormatUtil.h"

namespace Common::Log
{
@@ -77,13 +79,17 @@ enum LOG_LEVELS
static const char LOG_LEVEL_TO_CHAR[7] = "-NEWID";

void GenericLogFmtImpl(LOG_LEVELS level, LOG_TYPE type, const char* file, int line,
std::string_view format, const fmt::format_args& args);
fmt::string_view format, const fmt::format_args& args);

template <typename... Args>
void GenericLogFmt(LOG_LEVELS level, LOG_TYPE type, const char* file, int line,
std::string_view format, const Args&... args)
template <std::size_t NumFields, typename S, typename... Args>
void GenericLogFmt(LOG_LEVELS level, LOG_TYPE type, const char* file, int line, const S& format,
const Args&... args)
{
GenericLogFmtImpl(level, type, file, line, format, fmt::make_format_args(args...));
static_assert(NumFields == sizeof...(args),
"Unexpected number of replacement fields in format string; did you pass too few or "
"too many arguments?");
GenericLogFmtImpl(level, type, file, line, format,
fmt::make_args_checked<Args...>(format, args...));
}

void GenericLog(LOG_LEVELS level, LOG_TYPE type, const char* file, int line, const char* fmt, ...)
@@ -137,11 +143,16 @@ void GenericLog(LOG_LEVELS level, LOG_TYPE type, const char* file, int line, con

// fmtlib capable API

#define GENERIC_LOG_FMT(t, v, ...) \
#define GENERIC_LOG_FMT(t, v, format, ...) \
do \
{ \
if (v <= MAX_LOGLEVEL) \
Common::Log::GenericLogFmt(v, t, __FILE__, __LINE__, __VA_ARGS__); \
{ \
/* Use a macro-like name to avoid shadowing warnings */ \
constexpr auto GENERIC_LOG_FMT_N = Common::CountFmtReplacementFields(format); \
Common::Log::GenericLogFmt<GENERIC_LOG_FMT_N>(v, t, __FILE__, __LINE__, FMT_STRING(format), \
##__VA_ARGS__); \
} \
} while (0)

#define ERROR_LOG_FMT(t, ...) \
2 Source/Core/Common/Logging/LogManager.cpp
View file
@@ -81,7 +81,7 @@ void GenericLog(LOG_LEVELS level, LOG_TYPE type, const char* file, int line, con
}

void GenericLogFmtImpl(LOG_LEVELS level, LOG_TYPE type, const char* file, int line,
std::string_view format, const fmt::format_args& args)
fmt::string_view format, const fmt::format_args& args)
{
auto* instance = LogManager::GetInstance();
if (instance == nullptr)
38 Source/Core/Common/MsgHandler.h
View file
@@ -4,11 +4,14 @@

#pragma once

#include <cstdint>
#include <string>
#include <utility>

#include <fmt/format.h>

#include "Common/FormatUtil.h"

namespace Common
{
// Message alerts
@@ -37,9 +40,12 @@ bool MsgAlert(bool yes_no, MsgType style, const char* format, ...)
bool MsgAlertFmtImpl(bool yes_no, MsgType style, fmt::string_view format,
const fmt::format_args& args);

template <typename... Args>
bool MsgAlertFmt(bool yes_no, MsgType style, fmt::string_view format, const Args&... args)
template <std::size_t NumFields, typename S, typename... Args>
bool MsgAlertFmt(bool yes_no, MsgType style, const S& format, const Args&... args)
{
static_assert(NumFields == sizeof...(args),
"Unexpected number of replacement fields in format string; did you pass too few or "
"too many arguments?");
return MsgAlertFmtImpl(yes_no, style, format, fmt::make_args_checked<Args...>(format, args...));
}

@@ -88,33 +94,41 @@ std::string FmtFormatT(const char* string, Args&&... args)

// Fmt-capable variants of the macros

#define GenericAlertFmt(yes_no, style, format, ...) \
[&] { \
/* Use a macro-like name to avoid shadowing warnings */ \
constexpr auto GENERIC_ALERT_FMT_N = Common::CountFmtReplacementFields(format); \
return Common::MsgAlertFmt<GENERIC_ALERT_FMT_N>(yes_no, style, FMT_STRING(format), \
##__VA_ARGS__); \
}()

#define SuccessAlertFmt(format, ...) \
Common::MsgAlertFmt(false, Common::MsgType::Information, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(false, Common::MsgType::Information, format, ##__VA_ARGS__)

#define PanicAlertFmt(format, ...) \
Common::MsgAlertFmt(false, Common::MsgType::Warning, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(false, Common::MsgType::Warning, format, ##__VA_ARGS__)

#define PanicYesNoFmt(format, ...) \
Common::MsgAlertFmt(true, Common::MsgType::Warning, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(true, Common::MsgType::Warning, format, ##__VA_ARGS__)

#define AskYesNoFmt(format, ...) \
Common::MsgAlertFmt(true, Common::MsgType::Question, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(true, Common::MsgType::Question, format, ##__VA_ARGS__)

#define CriticalAlertFmt(format, ...) \
Common::MsgAlertFmt(false, Common::MsgType::Critical, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(false, Common::MsgType::Critical, format, ##__VA_ARGS__)

// Use these macros (that do the same thing) if the message should be translated.
#define SuccessAlertFmtT(format, ...) \
Common::MsgAlertFmt(false, Common::MsgType::Information, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(false, Common::MsgType::Information, format, ##__VA_ARGS__)

#define PanicAlertFmtT(format, ...) \
Common::MsgAlertFmt(false, Common::MsgType::Warning, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(false, Common::MsgType::Warning, format, ##__VA_ARGS__)

#define PanicYesNoFmtT(format, ...) \
Common::MsgAlertFmt(true, Common::MsgType::Warning, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(true, Common::MsgType::Warning, format, ##__VA_ARGS__)

#define AskYesNoFmtT(format, ...) \
Common::MsgAlertFmt(true, Common::MsgType::Question, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(true, Common::MsgType::Question, format, ##__VA_ARGS__)

#define CriticalAlertFmtT(format, ...) \
Common::MsgAlertFmt(false, Common::MsgType::Critical, FMT_STRING(format), ##__VA_ARGS__)
GenericAlertFmt(false, Common::MsgType::Critical, format, ##__VA_ARGS__)

0 comments on commit 419dfe4

Please sign in to comment.