From b3871dc8049819ae3e095555f451457567eb4ee3 Mon Sep 17 00:00:00 2001 From: Kai Koehne Date: Fri, 21 Feb 2014 11:39:12 +0100 Subject: [PATCH] Make parsing of categories in logging rules consistent. The documentation says that the left side of a logging rule has the syntax [.] with optional wildcard '*' as the first or the last character (or at both positions. However, so far we didn't allow qt.*.debug But what we did allow is implicit dropping of trailing '.', e.g. qt.* matched also 'qt' Fix these by splitting up the '.type' in advance, and then do string matching only on the 'real' category names. Change-Id: Iab50ad0fc673464e870f5ab8dfb3245d829b3107 Reviewed-by: Thiago Macieira --- src/corelib/io/qloggingregistry.cpp | 90 +++++++------- src/corelib/io/qloggingregistry_p.h | 7 +- .../qloggingregistry/tst_qloggingregistry.cpp | 114 ++++++++++++++++++ 3 files changed, 162 insertions(+), 49 deletions(-) diff --git a/src/corelib/io/qloggingregistry.cpp b/src/corelib/io/qloggingregistry.cpp index 2619743ff4..20eb2e13f5 100644 --- a/src/corelib/io/qloggingregistry.cpp +++ b/src/corelib/io/qloggingregistry.cpp @@ -65,11 +65,11 @@ QLoggingRule::QLoggingRule() : Constructs a logging rule. */ QLoggingRule::QLoggingRule(const QString &pattern, bool enabled) : - pattern(pattern), + messageType(-1), flags(Invalid), enabled(enabled) { - parse(); + parse(pattern); } /*! @@ -77,48 +77,33 @@ QLoggingRule::QLoggingRule(const QString &pattern, bool enabled) : Return value 1 means filter passed, 0 means filter doesn't influence this category, -1 means category doesn't pass this filter. */ -int QLoggingRule::pass(const QString &categoryName, QtMsgType msgType) const +int QLoggingRule::pass(const QString &cat, QtMsgType msgType) const { - QString fullCategory = categoryName; - switch (msgType) { - case QtDebugMsg: - fullCategory += QLatin1String(".debug"); - break; - case QtWarningMsg: - fullCategory += QLatin1String(".warning"); - break; - case QtCriticalMsg: - fullCategory += QLatin1String(".critical"); - break; - default: - break; - } + // check message type + if (messageType > -1 && messageType != msgType) + return 0; if (flags == FullText) { - // can be - // qtproject.org.debug = true - // or - // qtproject.org = true - if (pattern == categoryName - || pattern == fullCategory) + // full match + if (category == cat) return (enabled ? 1 : -1); + else + return 0; } - int idx = 0; - if (flags == MidFilter) { - // e.g. *.qtproject* - idx = fullCategory.indexOf(pattern); - if (idx >= 0) - return (enabled ? 1 : -1); - } else { - idx = fullCategory.indexOf(pattern); - if (flags == LeftFilter) { - // e.g. org.qtproject.* + const int idx = cat.indexOf(category); + if (idx >= 0) { + if (flags == MidFilter) { + // matches somewhere + if (idx >= 0) + return (enabled ? 1 : -1); + } else if (flags == LeftFilter) { + // matches left if (idx == 0) return (enabled ? 1 : -1); } else if (flags == RightFilter) { - // e.g. *.qtproject - if (idx == (fullCategory.count() - pattern.count())) + // matches right + if (idx == (cat.count() - category.count())) return (enabled ? 1 : -1); } } @@ -127,28 +112,41 @@ int QLoggingRule::pass(const QString &categoryName, QtMsgType msgType) const /*! \internal - Parses the category and checks which kind of wildcard the filter can contain. + Parses \a pattern. Allowed is f.ex.: - org.qtproject.logging FullText - org.qtproject.* LeftFilter - *.qtproject RightFilter - *.qtproject* MidFilter + qt.core.io.debug FullText, QtDebugMsg + qt.core.* LeftFilter, all types + *.io.warning RightFilter, QtWarningMsg + *.core.* MidFilter */ -void QLoggingRule::parse() +void QLoggingRule::parse(const QString &pattern) { - int index = pattern.indexOf(QLatin1Char('*')); + category = pattern; + // strip trailing ".messagetype" + if (pattern.endsWith(QLatin1String(".debug"))) { + category.chop(strlen(".debug")); + messageType = QtDebugMsg; + } else if (pattern.endsWith(QLatin1String(".warning"))) { + category.chop(strlen(".warning")); + messageType = QtWarningMsg; + } else if (pattern.endsWith(QLatin1String(".critical"))) { + category.chop(strlen(".critical")); + messageType = QtCriticalMsg; + } + + int index = category.indexOf(QLatin1Char('*')); if (index < 0) { flags = FullText; } else { flags = Invalid; if (index == 0) { flags |= RightFilter; - pattern = pattern.remove(0, 1); - index = pattern.indexOf(QLatin1Char('*')); + category.remove(0, 1); + index = category.indexOf(QLatin1Char('*')); } - if (index == (pattern.length() - 1)) { + if (index == (category.length() - 1)) { flags |= LeftFilter; - pattern = pattern.remove(pattern.length() - 1, 1); + category.chop(1); } } } diff --git a/src/corelib/io/qloggingregistry_p.h b/src/corelib/io/qloggingregistry_p.h index d4b97d42b8..70d728228c 100644 --- a/src/corelib/io/qloggingregistry_p.h +++ b/src/corelib/io/qloggingregistry_p.h @@ -64,7 +64,7 @@ class tst_QLoggingRegistry; QT_BEGIN_NAMESPACE -class QLoggingRule +class Q_AUTOTEST_EXPORT QLoggingRule { public: QLoggingRule(); @@ -80,12 +80,13 @@ public: }; Q_DECLARE_FLAGS(PatternFlags, PatternFlag) - QString pattern; + QString category; + int messageType; PatternFlags flags; bool enabled; private: - void parse(); + void parse(const QString &pattern); }; Q_DECLARE_OPERATORS_FOR_FLAGS(QLoggingRule::PatternFlags) diff --git a/tests/auto/corelib/io/qloggingregistry/tst_qloggingregistry.cpp b/tests/auto/corelib/io/qloggingregistry/tst_qloggingregistry.cpp index dc6f16828b..56545bb909 100644 --- a/tests/auto/corelib/io/qloggingregistry/tst_qloggingregistry.cpp +++ b/tests/auto/corelib/io/qloggingregistry/tst_qloggingregistry.cpp @@ -45,6 +45,13 @@ #include QT_USE_NAMESPACE +enum LoggingRuleState { + Invalid, + Match, + NoMatch +}; +Q_DECLARE_METATYPE(LoggingRuleState); +Q_DECLARE_METATYPE(QtMsgType); class tst_QLoggingRegistry : public QObject { @@ -59,6 +66,113 @@ private slots: qunsetenv("QT_LOGGING_CONF"); } + void QLoggingRule_parse_data() + { + QTest::addColumn("pattern"); + QTest::addColumn("category"); + QTest::addColumn("msgType"); + QTest::addColumn("result"); + + // _empty_ should match (only) _empty_ + QTest::newRow("_empty_-_empty_") + << QString("") << QString("") << QtDebugMsg << Match; + QTest::newRow("_empty_-default") + << QString("") << QString("default") << QtDebugMsg << NoMatch; + QTest::newRow(".debug-_empty_") + << QString(".debug") << QString("") << QtDebugMsg << Match; + QTest::newRow(".warning-default") + << QString(".warning") << QString("default") << QtDebugMsg << NoMatch; + + // literal should match only literal + QTest::newRow("qt-qt") + << QString("qt") << QString("qt") << QtDebugMsg << Match; + QTest::newRow("qt-_empty_") + << QString("qt") << QString("") << QtDebugMsg << NoMatch; + QTest::newRow("qt-qtx") + << QString("qt") << QString("qtx") << QtDebugMsg << NoMatch; + QTest::newRow("qt-qt.io") + << QString("qt") << QString("qt.io") << QtDebugMsg << NoMatch; + QTest::newRow("qt.debug-qt") + << QString("qt.debug") << QString("qt") << QtDebugMsg << Match; + QTest::newRow("qt.critical-qt") + << QString("qt.critical") << QString("qt") << QtDebugMsg << NoMatch; + + // * should match everything + QTest::newRow("_star_-qt.io.debug") + << QString("*") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("_star_-qt.io.warning") + << QString("*") << QString("qt.io") << QtWarningMsg << Match; + QTest::newRow("_star_-qt.io.critical") + << QString("*") << QString("qt.io") << QtCriticalMsg << Match; + QTest::newRow("_star_-_empty_") + << QString("*") << QString("") << QtDebugMsg << Match; + QTest::newRow("_star_.debug-qt.io") + << QString("*.debug") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("_star_.warning-qt.io") + << QString("*.warning") << QString("qt.io") << QtDebugMsg << NoMatch; + + // qt.* should match everything starting with 'qt.' + QTest::newRow("qt._star_-qt.io") + << QString("qt.*") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("qt._star_-qt") + << QString("qt.*") << QString("qt") << QtDebugMsg << NoMatch; + QTest::newRow("qt__star_-qt") + << QString("qt*") << QString("qt") << QtDebugMsg << Match; + QTest::newRow("qt._star_-qt.io.fs") + << QString("qt.*") << QString("qt.io.fs") << QtDebugMsg << Match; + QTest::newRow("qt._star_.debug-qt.io.fs") + << QString("qt.*.debug") << QString("qt.io.fs") << QtDebugMsg << Match; + QTest::newRow("qt._star_.warning-qt.io.fs") + << QString("qt.*.warning") << QString("qt.io.fs") << QtDebugMsg << NoMatch; + + // *.io should match everything ending with .io + QTest::newRow("_star_.io-qt.io") + << QString("*.io") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("_star_io-qt.io") + << QString("*io") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("_star_.io-io") + << QString("*.io") << QString("io") << QtDebugMsg << NoMatch; + QTest::newRow("_star_io-io") + << QString("*io") << QString("io") << QtDebugMsg << Match; + QTest::newRow("_star_.io-qt.ios") + << QString("*.io") << QString("qt.ios") << QtDebugMsg << NoMatch; + QTest::newRow("_star_.io-qt.io.x") + << QString("*.io") << QString("qt.io.x") << QtDebugMsg << NoMatch; + QTest::newRow("_star_.io.debug-qt.io") + << QString("*.io.debug") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("_star_.io.warning-qt.io") + << QString("*.io.warning") << QString("qt.io") << QtDebugMsg << NoMatch; + + // *qt* should match everything that contains 'qt' + QTest::newRow("_star_qt_star_-qt.core.io") + << QString("*qt*") << QString("qt.core.io") << QtDebugMsg << Match; + QTest::newRow("_star_qt_star_-default") + << QString("*qt*") << QString("default") << QtDebugMsg << NoMatch; + QTest::newRow("_star_qt._star_.debug-qt.io") + << QString("*qt.*.debug") << QString("qt.io") << QtDebugMsg << Match; + QTest::newRow("_star_.qt._star_.warning-qt.io") + << QString("*.qt.*.warning") << QString("qt.io") << QtDebugMsg << NoMatch; + } + + void QLoggingRule_parse() + { + QFETCH(QString, pattern); + QFETCH(QString, category); + QFETCH(QtMsgType, msgType); + QFETCH(LoggingRuleState, result); + + QLoggingRule rule(pattern, true); + LoggingRuleState state = Invalid; + if (rule.flags != QLoggingRule::Invalid) { + switch (rule.pass(category, msgType)) { + case -1: QFAIL("Shoudn't happen, we set pattern to true"); break; + case 0: state = NoMatch; break; + case 1: state = Match; break; + } + } + QCOMPARE(state, result); + } + void QLoggingSettingsParser_iniStyle() { //