[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: KDE/kdelibs/kdecore
From:       David Faure <faure () kde ! org>
Date:       2010-11-04 23:10:21
Message-ID: 201011050010.22283.faure () kde ! org
[Download RAW message or body]

On Thursday 04 November 2010, Oswald Buddenhagen wrote:
> On Thu, Nov 04, 2010 at 08:09:38PM +0100, David Faure wrote:
> > SVN commit 1193132 by dfaure:
> > +KDebug::Block::~Block()
> > +{
> > +   
> > kDebug_data->m_indentString.truncate(kDebug_data->m_indentString.length(
> > ) - 2); +
> 
> kDebug_data->m_indentString.chop(2);

Thanks, didn't know that one.

> > +class KDECORE_EXPORT KDebug::Block
> 
> i think you are supposed to provide an efficient no-op implementation
> for the KDE_NO_DEBUG_OUTPUT case.

Good point. 
Easy for that case (disabled at compile time), harder when disabled at runtime \
instead (hasNullOutput).

I played with a QScopedPointer in the KDEBUG_BLOCK macro, to only allocate the Block \
instance if needed, but that seemed wrong (to save an object on the stack when \
debugging is disabled-at-runtime, we pay the price of a new+delete when it's \
enabled). Instead I just turned the Block instance to a noop if hasNullOutput.

What do you think of the attached patch?

-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. Konqueror (http://www.konqueror.org).


["kdebug.diff" (text/x-patch)]

Index: io/kdebug.cpp
===================================================================
--- io/kdebug.cpp	(revision 1193141)
+++ io/kdebug.cpp	(working copy)
@@ -858,29 +858,35 @@
 KDebug::Block::Block(const char* label, int area)
     : m_label(label), m_area(area), d(0)
 {
-    m_startTime.start();
-    kDebug(area) << "BEGIN:" << label;
-    QMutexLocker locker(&kDebug_data->mutex);
-    kDebug_data->m_indentString += QLatin1String("  ");
+    if (hasNullOutputQtDebugMsg(area)) {
+        m_label = 0; // remember, for the dtor
+    } else {
+        m_startTime.start();
+        kDebug(area) << "BEGIN:" << label;
+        QMutexLocker locker(&kDebug_data->mutex);
+        kDebug_data->m_indentString += QLatin1String("  ");
+    }
 }
 
 KDebug::Block::~Block()
 {
-    const double duration = (double)m_startTime.elapsed() / (double)1000.0;
-    kDebug_data->mutex.lock();
-    kDebug_data->m_indentString.truncate(kDebug_data->m_indentString.length() - 2);
-    kDebug_data->mutex.unlock();
+    if (m_label) {
+        const double duration = (double)m_startTime.elapsed() / (double)1000.0;
+        kDebug_data->mutex.lock();
+        kDebug_data->m_indentString.truncate(kDebug_data->m_indentString.length() - \
2); +        kDebug_data->mutex.unlock();
 
-    // Print timing information, and a special message (DELAY) if the method took \
                longer than 5s
-    if (duration < 5.0) {
-        kDebug(m_area)
-            << "END__:"
-            << m_label
-            << QString::fromLatin1("[Took: %3s]").arg(QString::number(duration, 'g', \
                2) );
-    } else {
-        kDebug(m_area)
-            << "END__:"
-            << m_label
-            << QString::fromLatin1("[DELAY Took (quite long) \
%3s]").arg(QString::number(duration, 'g', 2)); +        // Print timing information, \
and a special message (DELAY) if the method took longer than 5s +        if (duration \
< 5.0) { +            kDebug(m_area)
+                << "END__:"
+                << m_label
+                << QString::fromLatin1("[Took: %3s]").arg(QString::number(duration, \
'g', 2) ); +        } else {
+            kDebug(m_area)
+                << "END__:"
+                << m_label
+                << QString::fromLatin1("[DELAY Took (quite long) \
%3s]").arg(QString::number(duration, 'g', 2)); +        }
     }
 }
Index: io/kdebug.h
===================================================================
--- io/kdebug.h	(revision 1193141)
+++ io/kdebug.h	(working copy)
@@ -365,7 +365,11 @@
 /**
  * Convenience macro for making a standard KDebug::Block
  */
-#define KDEBUG_BLOCK KDebug::Block \
uniquelyNamedStackAllocatedStandardBlock(Q_FUNC_INFO); +#ifdef KDE_NO_DEBUG_OUTPUT
+#define KDEBUG_BLOCK
+#else
+#define KDEBUG_BLOCK KDebug::Block _kDebugBlock(Q_FUNC_INFO);
+#endif
 
 /**
  * Convenience macro, use this to remind yourself to finish the implementation of a \
function



[prev in list] [next in list] [prev in thread] [next in thread]