Skip to content

Commit

Permalink
logging: Fix easylogging++ init with blank config
Browse files Browse the repository at this point in the history
The upstream version of el::base::TypedConfigurations::unsafeGetConfigByRef
accesses uninitialized memory if a key doesn't exist.
Commit b2c59af patched the library to
throw in this case, avoiding the invalid access, but the more suitable
pattern, both logically, and as evidenced by the behavior of
unsafeGetConfigByVal, would be to return a const reference to a
default-initialized value with static storage duration.
  • Loading branch information
iamamyth committed Feb 19, 2025
1 parent 915c5dc commit 8c43e33
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 5 deletions.
7 changes: 4 additions & 3 deletions external/easylogging++/easylogging++.h
Original file line number Diff line number Diff line change
Expand Up @@ -1993,7 +1993,7 @@ class TypedConfigurations : public base::threading::ThreadSafe {
}

template <typename Conf_T>
inline Conf_T& getConfigByRef(Level level, std::unordered_map<Level, Conf_T>* confMap, const char* confName) {
inline const Conf_T& getConfigByRef(Level level, std::unordered_map<Level, Conf_T>* confMap, const char* confName) {
base::threading::ScopedLock scopedLock(lock());
return unsafeGetConfigByRef(level, confMap, confName); // This is not unsafe anymore - mutex locked in scope
}
Expand All @@ -2016,8 +2016,9 @@ class TypedConfigurations : public base::threading::ThreadSafe {
}

template <typename Conf_T>
Conf_T& unsafeGetConfigByRef(Level level, std::unordered_map<Level, Conf_T>* confMap, const char* confName) {
const Conf_T& unsafeGetConfigByRef(Level level, std::unordered_map<Level, Conf_T>* confMap, const char* confName) {
ELPP_UNUSED(confName);
static const Conf_T empty;
typename std::unordered_map<Level, Conf_T>::iterator it = confMap->find(level);
if (it == confMap->end()) {
try {
Expand All @@ -2026,7 +2027,7 @@ class TypedConfigurations : public base::threading::ThreadSafe {
ELPP_INTERNAL_ERROR("Unable to get configuration [" << confName << "] for level ["
<< LevelHelper::convertToString(level) << "]"
<< std::endl << "Please ensure you have properly configured logger.", false);
throw; // The exception has to be rethrown, to abort a branch leading to UB.
return empty;
}
}
return it->second;
Expand Down
5 changes: 3 additions & 2 deletions tests/unit_tests/logging.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,10 @@ TEST(logging, operator_equals_segfault)
log2 = log1;
}

TEST(logging, empty_configurations_throws)
TEST(logging, empty_configuration)
{
el::Logger log1("id1", nullptr);
const el::Configurations cfg;
EXPECT_ANY_THROW(log1.configure(cfg));
EXPECT_NO_THROW(log1.configure(cfg));
EXPECT_EQ(log1.typedConfigurations()->filename(el::Level::Info), "");
}

0 comments on commit 8c43e33

Please sign in to comment.