diff --git a/rts/System/Log/DefaultFilter.cpp b/rts/System/Log/DefaultFilter.cpp index cf0c3939d29..3ec33410cf1 100644 --- a/rts/System/Log/DefaultFilter.cpp +++ b/rts/System/Log/DefaultFilter.cpp @@ -157,13 +157,15 @@ void log_filter_section_setMinLevel(int level, const char* section) // (same string but will not become garbage) section = *registeredSection; - if (level == log_filter_section_getDefaultMinLevel(section)) { - using P = decltype(log_filter::sectionMinLevels)::value_type; - - const auto sectionComparer = [](const P& a, const P& b) { return (log_filter_section_compare()(a.first, b.first)); }; - const auto sectionMinLevel = std::lower_bound(secLvls.begin(), secLvls.begin() + log_filter::numLevels, P{section, 0}, sectionComparer); + // locate any existing override for this section (the array is kept sorted) + using P = decltype(log_filter::sectionMinLevels)::value_type; + const auto sectionComparer = [](const P& a, const P& b) { return (log_filter_section_compare()(a.first, b.first)); }; + const auto sectionMinLevel = std::lower_bound(secLvls.begin(), secLvls.begin() + log_filter::numLevels, P{section, 0}, sectionComparer); + const bool exists = (sectionMinLevel != (secLvls.begin() + log_filter::numLevels) && strcmp(sectionMinLevel->first, section) == 0); - if (sectionMinLevel == (secLvls.begin() + log_filter::numLevels) || strcmp(sectionMinLevel->first, section) != 0) + if (level == log_filter_section_getDefaultMinLevel(section)) { + // back to default: drop any existing override (nothing to do otherwise) + if (!exists) return; // erase @@ -175,6 +177,13 @@ void log_filter_section_setMinLevel(int level, const char* section) return; } + // non-default: update any existing override in-place + if (exists) { + sectionMinLevel->second = level; + return; + } + + // add a net new override secLvls[log_filter::numLevels++] = {section, level}; // swap into position diff --git a/test/engine/System/Log/TestILog.cpp b/test/engine/System/Log/TestILog.cpp index cb8d4b3e4a1..7b7d1345f0c 100644 --- a/test/engine/System/Log/TestILog.cpp +++ b/test/engine/System/Log/TestILog.cpp @@ -3,6 +3,7 @@ #include "System/Log/FileSink.h" #include "System/Log/StreamSink.h" #include "System/Log/LogUtil.h" +#include "System/Log/DefaultFilter.h" #include @@ -231,3 +232,33 @@ TEST_CASE("IsEnabled") TLOG_SL( "other-one-time-section", L_DEBUG, "Testing LOG_IS_ENABLED_S"); } + +// Regression for the duplicate-entry leak in log_filter_section_setMinLevel. +// Setting a section to a non-default level used to *append* a new row every call +// instead of updating the existing one, so repeated changes to one section filled +// the fixed-size sectionMinLevels table and then made *all* section-level changes +// silently fail ("too many section-levels"). +TEST_CASE("SectionMinLevelNoDuplicateLeak") +{ + // non-default levels for these (non-default) sections; restored at the end + const int savedDefined = log_filter_section_getMinLevel(LOG_SECTION_DEFINED); + const int savedOneTime = log_filter_section_getMinLevel(LOG_SECTION_ONE_TIME_0); + + // hammer one section far more than the table could ever hold + for (int i = 0; i < 300; ++i) + log_filter_section_setMinLevel((i & 1) ? LOG_LEVEL_WARNING : LOG_LEVEL_ERROR, LOG_SECTION_DEFINED); + + // the most recent value wins (a single, updated-in-place entry) + log_filter_section_setMinLevel(LOG_LEVEL_ERROR, LOG_SECTION_DEFINED); + CHECK(log_filter_section_getMinLevel(LOG_SECTION_DEFINED) == LOG_LEVEL_ERROR); + + // and a *different* section must still be settable: with the old append bug + // the table is saturated by now and this set would be dropped + log_filter_section_setMinLevel(LOG_LEVEL_WARNING, LOG_SECTION_ONE_TIME_0); + CHECK(log_filter_section_getMinLevel(LOG_SECTION_ONE_TIME_0) == LOG_LEVEL_WARNING); + + // restore original levels (setting back to default takes the erase path) + log_filter_section_setMinLevel(savedDefined, LOG_SECTION_DEFINED); + log_filter_section_setMinLevel(savedOneTime, LOG_SECTION_ONE_TIME_0); +} +