On 19/02/2013 9:22 a.m., Alex Rousskov wrote:
On 02/18/2013 04:39 AM, Amos Jeffries wrote:
On 10/08/2012 1:57 p.m., Amos Jeffries wrote:
On 9/08/2012 12:12 a.m., Alexander Komyagin wrote:
Following patch fixes nested debug() calls problem.
Since debugs() is a macros, it should not change static Debugs::level
before putting debug message to the internal stream. Otherwise we
encounter problems when debug message itself contains calls to debugs().
--- src/Debug.h 2012-03-07 05:42:55.000000000 +0300
+++ src/Debug.h 2012-08-08 14:49:20.000000000 +0400
@@ -106,8 +106,9 @@
/* Debug stream */
#define debugs(SECTION, LEVEL, CONTENT) \
do { \
- if ((Debug::level = (LEVEL)) <= Debug::Levels[SECTION]) { \
+ if ((LEVEL) <= Debug::Levels[SECTION]) { \
Debug::getDebugOut() << CONTENT; \
+ Debug::level = (LEVEL); \
Debug::finishDebug(); \
} \
} while (/*CONSTCOND*/ 0)
Looks okay. How much testing has this had?
Amos
Applied as trunk rev.12698. Sorry this took so long to go in.
Please note that debugs() is not the only macro that changes
Debug::level before doing anything else. The old do_debug() does that as
well.
debugs() still changes Debug::sectionLevel before we do anything else so
there is inconsistency now with regard to level and sectionLevel.
Finally, this change makes current debugging level unavailable to
CONTENT. I know some CONTENT uses Debug::sectionLevel. I do not see any
CONTENT using Debug::level, so this may not be a problem today.
I cannot tell exactly what problems this change is fixing (the commit
message does not explain), so the fix may very well be worth the
side-effects, but it looks like the debugging code became messier after
this change. I wonder if the fix is worth it and whether there is a
better way to fix that problem?
Possibly. Reverting. I applied because it passed the audit conditions
months back but had not been applied by anyone yet.
Amos
