compilerplugins/clang/stringconcatliterals.cxx | 3 -
include/sal/log.hxx | 61 ++++++++++++++++++-------
sal/osl/all/log.cxx | 39 ++++++++++++---
3 files changed, 78 insertions(+), 25 deletions(-)
New commits:
commit b9d93fc47b2489764e251a11572fccef872df4e9
Author: Jan-Marek Glogowski <glo...@fbihome.de>
AuthorDate: Wed Jul 1 18:06:28 2020 +0200
Commit: Thorsten Behrens <thorsten.behr...@cib.de>
CommitDate: Tue Jul 7 14:15:04 2020 +0200
Allow making SAL_LOG based output fatal
This introduces the [+-]FATAL marker for SAL_LOG. This way you can
set part of the matching SAL_LOG string to std::abort on match.
The example "SAL_LOG=+FATAL+WARN.vcl.scheduler-FATAL+INFO" will
abort for any "+WARN.vcl.scheduler" match, but will just print all
additional "+INFO" logs.
Change-Id: Ib77f194a78f5165e6c885c82374ae41293815ee9
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/97651
Tested-by: Jenkins
Reviewed-by: Thorsten Behrens <thorsten.behr...@cib.de>
diff --git a/compilerplugins/clang/stringconcatliterals.cxx
b/compilerplugins/clang/stringconcatliterals.cxx
index 0f26f4f553bc..0b52bd0c9b4f 100644
--- a/compilerplugins/clang/stringconcatliterals.cxx
+++ b/compilerplugins/clang/stringconcatliterals.cxx
@@ -109,7 +109,8 @@ bool StringConcatLiterals::VisitCallExpr(CallExpr const *
expr) {
compiler.getSourceManager().getSpellingLoc(
compiler.getSourceManager().getImmediateMacroCallerLoc(
compiler.getSourceManager().getImmediateMacroCallerLoc(
- compat::getBeginLoc(expr))))),
+
compiler.getSourceManager().getImmediateMacroCallerLoc(
+ compat::getBeginLoc(expr)))))),
SRCDIR "/include/tools/diagnose_ex.h"))
return true;
diff --git a/include/sal/log.hxx b/include/sal/log.hxx
index 00d533ab5495..6bb0d1b43d3d 100644
--- a/include/sal/log.hxx
+++ b/include/sal/log.hxx
@@ -26,11 +26,20 @@
/// @cond INTERNAL
+enum sal_detail_LogAction
+{
+ SAL_DETAIL_LOG_ACTION_IGNORE,
+ SAL_DETAIL_LOG_ACTION_LOG,
+ SAL_DETAIL_LOG_ACTION_FATAL
+};
+
extern "C" SAL_DLLPUBLIC void SAL_CALL sal_detail_log(
sal_detail_LogLevel level, char const * area, char const * where,
char const * message, sal_uInt32 backtraceDepth);
-extern "C" SAL_DLLPUBLIC sal_Bool SAL_CALL sal_detail_log_report(
+// the return value is actually "enum sal_detail_LogAction", but due to ABI
+// compatibility, it's left as the original "sal_Bool" / "unsigned char".
+extern "C" SAL_DLLPUBLIC unsigned char SAL_CALL sal_detail_log_report(
sal_detail_LogLevel level, char const * area);
namespace sal { namespace detail {
@@ -113,22 +122,38 @@ inline char const * unwrapStream(SAL_UNUSED_PARAMETER
StreamIgnore const &) {
} }
+// to prevent using a local variable, which can eventually shadow,
+// resulting in compiler warnings (or even errors with -Werror)
+#define SAL_DETAIL_LOG_STREAM_PRIVATE_(level, area, where, stream) \
+ if (sizeof ::sal::detail::getResult( \
+ ::sal::detail::StreamStart() << stream) == 1) \
+ { \
+ ::sal_detail_log( \
+ (level), (area), (where), \
+ ::sal::detail::unwrapStream( \
+ ::sal::detail::StreamStart() << stream), \
+ 0); \
+ } else { \
+ ::std::ostringstream sal_detail_stream; \
+ sal_detail_stream << stream; \
+ ::sal::detail::log( \
+ (level), (area), (where), sal_detail_stream, 0); \
+ }
+
#define SAL_DETAIL_LOG_STREAM(condition, level, area, where, stream) \
do { \
- if ((condition) && sal_detail_log_report(level, area)) { \
- if (sizeof ::sal::detail::getResult( \
- ::sal::detail::StreamStart() << stream) == 1) \
+ if (condition) \
+ { \
+ switch (sal_detail_log_report(level, area)) \
{ \
- ::sal_detail_log( \
- (level), (area), (where), \
- ::sal::detail::unwrapStream( \
- ::sal::detail::StreamStart() << stream), \
- 0); \
- } else { \
- ::std::ostringstream sal_detail_stream; \
- sal_detail_stream << stream; \
- ::sal::detail::log( \
- (level), (area), (where), sal_detail_stream, 0); \
+ case SAL_DETAIL_LOG_ACTION_IGNORE: break; \
+ case SAL_DETAIL_LOG_ACTION_LOG: \
+ SAL_DETAIL_LOG_STREAM_PRIVATE_(level, area, where, stream); \
+ break; \
+ case SAL_DETAIL_LOG_ACTION_FATAL: \
+ SAL_DETAIL_LOG_STREAM_PRIVATE_(level, area, where, stream); \
+ std::abort(); \
+ break; \
} \
} \
} while (false)
@@ -235,7 +260,7 @@ inline char const * unwrapStream(SAL_UNUSED_PARAMETER
StreamIgnore const &) {
<switch> ::= <sense><item>
<sense> ::= "+"|"-"
<item> ::= <flag>|<level>("."<area>)?
- <flag> ::= "TIMESTAMP"|"RELATIVETIMER"
+ <flag> ::= "TIMESTAMP"|"RELATIVETIMER"|"FATAL"
<level> ::= "INFO"|"WARN"
@endverbatim
@@ -252,6 +277,12 @@ inline char const * unwrapStream(SAL_UNUSED_PARAMETER
StreamIgnore const &) {
the level switch(es)) to be prefixed by a relative timestamp in
seconds since the first output line like 1.312.
+ The "+FATAL" flag will cause later matching rules to log and call
+ std::abort. This can be disabled at some later point by using the
+ "-FATAL" flag before specifying additional rules. The flag will just
+ abort on positive rules, as it doesn't seem to make sense to abort
+ on ignored output.
+
If both +TIMESTAMP and +RELATIVETIMER are specified, they are
output in that order.
diff --git a/sal/osl/all/log.cxx b/sal/osl/all/log.cxx
index ed663076b8d2..927e78b97064 100644
--- a/sal/osl/all/log.cxx
+++ b/sal/osl/all/log.cxx
@@ -344,7 +344,9 @@ void sal_detail_logFormat(
sal_detail_LogLevel level, char const * area, char const * where,
char const * format, ...)
{
- if (!sal_detail_log_report(level, area))
+ const sal_detail_LogAction eAction
+ = static_cast<sal_detail_LogAction>(sal_detail_log_report(level,
area));
+ if (eAction == SAL_DETAIL_LOG_ACTION_IGNORE)
return;
std::va_list args;
@@ -359,11 +361,15 @@ void sal_detail_logFormat(
}
sal_detail_log(level, area, where, buf, 0);
va_end(args);
+
+ if (eAction == SAL_DETAIL_LOG_ACTION_FATAL)
+ std::abort();
}
-sal_Bool sal_detail_log_report(sal_detail_LogLevel level, char const * area) {
+unsigned char sal_detail_log_report(sal_detail_LogLevel level, char const *
area)
+{
if (level == SAL_DETAIL_LOG_LEVEL_DEBUG) {
- return true;
+ return SAL_DETAIL_LOG_ACTION_LOG;
}
assert(area != nullptr);
static char const* const env = [] {
@@ -379,17 +385,26 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level,
char const * area) {
// no matching switches at all, the result will be negative (and
// initializing with 1 is safe as the length of a valid switch, even
// without the "+"/"-" prefix, will always be > 1)
+ bool senseFatal[2] = { false, false };
bool seenWarn = false;
+ bool bFlagFatal = false;
for (char const * p = env;;) {
Sense sense;
switch (*p++) {
case '\0':
+ {
if (level == SAL_DETAIL_LOG_LEVEL_WARN && !seenWarn)
return sal_detail_log_report(SAL_DETAIL_LOG_LEVEL_INFO, area);
- return senseLen[POSITIVE] >= senseLen[NEGATIVE];
- // if a specific item is both positive and negative
- // (senseLen[POSITIVE] == senseLen[NEGATIVE]), default to
- // positive
+
+ sal_detail_LogAction eAction = SAL_DETAIL_LOG_ACTION_IGNORE;
+ // if a specific item is positive and negative (==), default to
positive
+ if (senseLen[POSITIVE] >= senseLen[NEGATIVE])
+ {
+ if (senseFatal[POSITIVE]) eAction =
SAL_DETAIL_LOG_ACTION_FATAL;
+ else eAction = SAL_DETAIL_LOG_ACTION_LOG;
+ }
+ return eAction;
+ }
case '+':
sense = POSITIVE;
break;
@@ -397,7 +412,7 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level,
char const * area) {
sense = NEGATIVE;
break;
default:
- return true; // upon an illegal SAL_LOG value, enable everything
+ return SAL_DETAIL_LOG_ACTION_LOG; // upon an illegal SAL_LOG
value, enable everything
}
char const * p1 = p;
while (*p1 != '.' && *p1 != '+' && *p1 != '-' && *p1 != '\0') {
@@ -410,13 +425,17 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level,
char const * area) {
{
match = level == SAL_DETAIL_LOG_LEVEL_WARN;
seenWarn = true;
+ } else if (equalStrings(p, p1 - p,
RTL_CONSTASCII_STRINGPARAM("FATAL")))
+ {
+ bFlagFatal = (sense == POSITIVE);
+ match = false;
} else if (equalStrings(p, p1 - p,
RTL_CONSTASCII_STRINGPARAM("TIMESTAMP")) ||
equalStrings(p, p1 - p,
RTL_CONSTASCII_STRINGPARAM("RELATIVETIMER")))
{
// handled later
match = false;
} else {
- return true;
+ return SAL_DETAIL_LOG_ACTION_LOG;
// upon an illegal SAL_LOG value, everything is considered
// positive
}
@@ -433,9 +452,11 @@ sal_Bool sal_detail_log_report(sal_detail_LogLevel level,
char const * area) {
&& equalStrings(p1, n, area, n)))
{
senseLen[sense] = p2 - p;
+ senseFatal[sense] = bFlagFatal;
}
} else {
senseLen[sense] = p1 - p;
+ senseFatal[sense] = bFlagFatal;
}
}
p = p2;
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
