From 6e758744986442088206fe59c449340cacf103ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 18 Feb 2017 10:54:01 +0100 Subject: [PATCH 1/4] Allow to break into the debugger instead of just abbort. --- src/library/baseplaylistfeature.cpp | 3 +++ src/util/assert.h | 11 +++++++++-- src/util/cmdlineargs.cpp | 13 ++++++++++++- src/util/cmdlineargs.h | 2 ++ 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/library/baseplaylistfeature.cpp b/src/library/baseplaylistfeature.cpp index 2cfd68e974f0..9d364c45b7c1 100644 --- a/src/library/baseplaylistfeature.cpp +++ b/src/library/baseplaylistfeature.cpp @@ -97,6 +97,9 @@ BasePlaylistFeature::BasePlaylistFeature(QObject* parent, this, SLOT(slotTrackSelected(TrackPointer))); connect(pLibrary, SIGNAL(switchToView(const QString&)), this, SLOT(slotResetSelectedTrack())); + + + DEBUG_ASSERT(false); } BasePlaylistFeature::~BasePlaylistFeature() { diff --git a/src/util/assert.h b/src/util/assert.h index 2ffec2edfea3..a1d4ae07e1c8 100644 --- a/src/util/assert.h +++ b/src/util/assert.h @@ -2,15 +2,22 @@ #define ASSERT_H #include +#include +#include inline void mixxx_noop(void) {} inline void mixxx_debug_assert(const char* assertion, const char* file, int line, const char* function) { + if (CmdlineArgs::Instance().getDebugAssertBreak()) { + qWarning("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); + raise(SIGINT); + } else { #ifdef MIXXX_DEBUG_ASSERTIONS_FATAL - qFatal("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); + qFatal("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); #else - qWarning("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); + qWarning("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); #endif + } } inline bool mixxx_maybe_debug_assert_return_true(const char* assertion, const char* file, int line, const char* function) { diff --git a/src/util/cmdlineargs.cpp b/src/util/cmdlineargs.cpp index 79421e2ca977..1f944cbb46f6 100644 --- a/src/util/cmdlineargs.cpp +++ b/src/util/cmdlineargs.cpp @@ -11,6 +11,7 @@ CmdlineArgs::CmdlineArgs() m_midiDebug(false), m_developer(false), m_safeMode(false), + m_debugAssertBreak(false), m_settingsPathSet(false), m_logLevel(mixxx::Logging::kLogLevelDefault), // We are not ready to switch to XDG folders under Linux, so keeping $HOME/.mixxx as preferences folder. see lp:1463273 @@ -77,6 +78,8 @@ warnings and errors to the console unless this is set properly.\n", stdout); m_developer = true; } else if (QString::fromLocal8Bit(argv[i]).contains("--safeMode", Qt::CaseInsensitive)) { m_safeMode = true; + } else if (QString::fromLocal8Bit(argv[i]).contains("--debugAssertBreak", Qt::CaseInsensitive)) { + m_debugAssertBreak = true; } else { m_musicFiles += QString::fromLocal8Bit(argv[i]); } @@ -142,7 +145,15 @@ void CmdlineArgs::printUsage() { warning - Above + Warnings\n\ info - Above + Informational messages\n\ debug - Above + Debug/Developer messages\n\ -\n\ +\n" +#ifdef MIXXX_BUILD_DEBUG +"\ +--debugAssertBreak Breaks (SIGINT) Mixxx, if a DEBUG_ASSERT\n\ + evaluates to false. Under a debugger you can\n\ + continue afterwards.\ +\n" +#endif +"\ -h, --help Display this help message and exit", stdout); fputs("\n\n(For more information, see http://mixxx.org/wiki/doku.php/command_line_options)\n",stdout); diff --git a/src/util/cmdlineargs.h b/src/util/cmdlineargs.h index 32d42b68d978..8c5ca1beed41 100644 --- a/src/util/cmdlineargs.h +++ b/src/util/cmdlineargs.h @@ -24,6 +24,7 @@ class CmdlineArgs final { bool getMidiDebug() const { return m_midiDebug; } bool getDeveloper() const { return m_developer; } bool getSafeMode() const { return m_safeMode; } + bool getDebugAssertBreak() const { return m_debugAssertBreak; } bool getSettingsPathSet() const { return m_settingsPathSet; } mixxx::Logging::LogLevel getLogLevel() const { return m_logLevel; } bool getTimelineEnabled() const { return !m_timelinePath.isEmpty(); } @@ -44,6 +45,7 @@ class CmdlineArgs final { bool m_midiDebug; bool m_developer; // Developer Mode bool m_safeMode; + bool m_debugAssertBreak; bool m_settingsPathSet; // has --settingsPath been set on command line ? mixxx::Logging::LogLevel m_logLevel; // Level of logging message verbosity QString m_locale; From 27bf71683e81792b77ae8d62578af7d30783499f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sat, 18 Feb 2017 22:37:58 +0100 Subject: [PATCH 2/4] Move debug assert behaviour out of the inlined header into the logging code --- src/util/assert.h | 13 +------------ src/util/logging.cpp | 41 ++++++++++++++++++++++++++++++++++++----- src/util/logging.h | 1 + 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/src/util/assert.h b/src/util/assert.h index a1d4ae07e1c8..5897d09c9b48 100644 --- a/src/util/assert.h +++ b/src/util/assert.h @@ -2,22 +2,11 @@ #define ASSERT_H #include -#include -#include inline void mixxx_noop(void) {} inline void mixxx_debug_assert(const char* assertion, const char* file, int line, const char* function) { - if (CmdlineArgs::Instance().getDebugAssertBreak()) { - qWarning("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); - raise(SIGINT); - } else { -#ifdef MIXXX_DEBUG_ASSERTIONS_FATAL - qFatal("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); -#else - qWarning("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); -#endif - } + qCritical("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); } inline bool mixxx_maybe_debug_assert_return_true(const char* assertion, const char* file, int line, const char* function) { diff --git a/src/util/logging.cpp b/src/util/logging.cpp index 95f304218ed0..d8cf88b2cc24 100644 --- a/src/util/logging.cpp +++ b/src/util/logging.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -132,11 +133,41 @@ void MessageHandler(QtMsgType type, } break; case QtCriticalMsg: - // Critical errors are always shown on the console. - fprintf(stderr, "Critical %s", ba.constData()); - if (Logfile.isOpen()) { - Logfile.write("Critical "); - Logfile.write(ba); + { +#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) + bool debugAssert = strncmp(input, Logging::kAssertPrefix, + strlen(Logging::kAssertPrefix)) == 0; +#else + bool debugAssert = input.startsWith(QLatin1String(Logging::kAssertPrefix)); +#endif + if (debugAssert) { + if (CmdlineArgs::Instance().getDebugAssertBreak()) { + fprintf(stderr, "%s", ba.constData()); + if (Logfile.isOpen()) { + Logfile.write(ba); + } + raise(SIGINT); + } else { + #ifdef MIXXX_DEBUG_ASSERTIONS_FATAL + // re-send as fatal + locker.unlock(); + qFatal("%s", input); + return; + #else + fprintf(stderr, "%s", ba.constData()); + if (Logfile.isOpen()) { + Logfile.write(ba); + } + #endif + } + } else { + // Critical errors are always shown on the console. + fprintf(stderr, "Critical %s", ba.constData()); + if (Logfile.isOpen()) { + Logfile.write("Critical "); + Logfile.write(ba); + } + } } break; case QtFatalMsg: diff --git a/src/util/logging.h b/src/util/logging.h index cec663559416..693cc4c792b3 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -15,6 +15,7 @@ class Logging { // Any debug statement starting with this prefix bypasses the --logLevel // command line flags. static constexpr const char* kControllerDebugPrefix = "CDBG"; + static constexpr const char* kAssertPrefix = "DEBUG ASSERT"; static void initialize(); static void shutdown(); From b36756c6c89c8150a687d69363ea310ad9e776bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Sun, 19 Feb 2017 01:16:15 +0100 Subject: [PATCH 3/4] Fix Qt5 build error --- src/library/baseplaylistfeature.cpp | 3 --- src/util/logging.cpp | 16 ++++++++++------ 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/library/baseplaylistfeature.cpp b/src/library/baseplaylistfeature.cpp index 9d364c45b7c1..2cfd68e974f0 100644 --- a/src/library/baseplaylistfeature.cpp +++ b/src/library/baseplaylistfeature.cpp @@ -97,9 +97,6 @@ BasePlaylistFeature::BasePlaylistFeature(QObject* parent, this, SLOT(slotTrackSelected(TrackPointer))); connect(pLibrary, SIGNAL(switchToView(const QString&)), this, SLOT(slotResetSelectedTrack())); - - - DEBUG_ASSERT(false); } BasePlaylistFeature::~BasePlaylistFeature() { diff --git a/src/util/logging.cpp b/src/util/logging.cpp index d8cf88b2cc24..a6ed85b6ddeb 100644 --- a/src/util/logging.cpp +++ b/src/util/logging.cpp @@ -142,23 +142,27 @@ void MessageHandler(QtMsgType type, #endif if (debugAssert) { if (CmdlineArgs::Instance().getDebugAssertBreak()) { - fprintf(stderr, "%s", ba.constData()); + fprintf(stderr, ba.constData()); if (Logfile.isOpen()) { Logfile.write(ba); } raise(SIGINT); } else { - #ifdef MIXXX_DEBUG_ASSERTIONS_FATAL +#ifdef MIXXX_DEBUG_ASSERTIONS_FATAL // re-send as fatal locker.unlock(); - qFatal("%s", input); +#if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) + qFatal(input); +#else + qFatal(input.toLocal8Bit()); +#endif return; - #else - fprintf(stderr, "%s", ba.constData()); +#else + fprintf(stderr, ba.constData()); if (Logfile.isOpen()) { Logfile.write(ba); } - #endif +#endif } } else { // Critical errors are always shown on the console. From 26599ffbd140c0efa6039d4485a1059cd28d8b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Sch=C3=BCrmann?= Date: Tue, 21 Feb 2017 22:39:17 +0100 Subject: [PATCH 4/4] use common definition for debug assert prefix --- src/util/assert.h | 4 +++- src/util/logging.cpp | 7 ++++--- src/util/logging.h | 1 - 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/util/assert.h b/src/util/assert.h index 5897d09c9b48..245ed6c3ad41 100644 --- a/src/util/assert.h +++ b/src/util/assert.h @@ -3,10 +3,12 @@ #include +static constexpr const char* kDebugAssertPrefix = "DEBUG ASSERT"; + inline void mixxx_noop(void) {} inline void mixxx_debug_assert(const char* assertion, const char* file, int line, const char* function) { - qCritical("DEBUG ASSERT: \"%s\" in function %s at %s:%d", assertion, function, file, line); + qCritical("%s: \"%s\" in function %s at %s:%d", kDebugAssertPrefix, assertion, function, file, line); } inline bool mixxx_maybe_debug_assert_return_true(const char* assertion, const char* file, int line, const char* function) { diff --git a/src/util/logging.cpp b/src/util/logging.cpp index a6ed85b6ddeb..4311b46870e8 100644 --- a/src/util/logging.cpp +++ b/src/util/logging.cpp @@ -16,6 +16,7 @@ #include #include "util/cmdlineargs.h" +#include "util/assert.h" namespace mixxx { namespace { @@ -135,10 +136,10 @@ void MessageHandler(QtMsgType type, case QtCriticalMsg: { #if QT_VERSION < QT_VERSION_CHECK(5, 0, 0) - bool debugAssert = strncmp(input, Logging::kAssertPrefix, - strlen(Logging::kAssertPrefix)) == 0; + bool debugAssert = strncmp(input, kDebugAssertPrefix, + strlen(kDebugAssertPrefix)) == 0; #else - bool debugAssert = input.startsWith(QLatin1String(Logging::kAssertPrefix)); + bool debugAssert = input.startsWith(QLatin1String(kDebugAssertPrefix)); #endif if (debugAssert) { if (CmdlineArgs::Instance().getDebugAssertBreak()) { diff --git a/src/util/logging.h b/src/util/logging.h index 693cc4c792b3..cec663559416 100644 --- a/src/util/logging.h +++ b/src/util/logging.h @@ -15,7 +15,6 @@ class Logging { // Any debug statement starting with this prefix bypasses the --logLevel // command line flags. static constexpr const char* kControllerDebugPrefix = "CDBG"; - static constexpr const char* kAssertPrefix = "DEBUG ASSERT"; static void initialize(); static void shutdown();