Skip to content
Browse files

TS-1102 Cleanup obsolete debugging code

Author: Uri Shachar
Review: leif (I change indentation in a few places, 2 space indent)

Thanks Uri, keep em coming!
  • Loading branch information...
1 parent 9289d73 commit 5ad005cb4bd16084615a3ade43271208dc6b374e @zwoop zwoop committed
Showing with 89 additions and 227 deletions.
  1. +2 −2 lib/records/RecUtils.cc
  2. +29 −51 lib/ts/Diags.cc
  3. +39 −154 lib/ts/Diags.h
  4. +1 −1 mgmt/api/CoreAPI.cc
  5. +10 −10 mgmt/utils/MgmtUtils.cc
  6. +5 −5 proxy/DiagsConfig.cc
  7. +1 −1 proxy/Error.cc
  8. +2 −2 proxy/InkAPI.cc
  9. +0 −1 proxy/UglyLogStubs.cc
View
4 lib/records/RecUtils.cc
@@ -230,7 +230,7 @@ RecLog(DiagsLevel dl, const char *format_string, ...)
va_start(ap, format_string);
if (g_diags) {
- g_diags->log_va(NULL, dl, NULL, NULL, format_string, ap);
+ g_diags->log_va(NULL, dl, NULL, format_string, ap);
}
va_end(ap);
}
@@ -246,7 +246,7 @@ RecDebug(DiagsLevel dl, const char *format_string, ...)
va_start(ap, format_string);
if (g_diags) {
- g_diags->log_va("rec", dl, NULL, NULL, format_string, ap);
+ g_diags->log_va("rec", dl, NULL, format_string, ap);
}
va_end(ap);
}
View
80 lib/ts/Diags.cc
@@ -164,8 +164,7 @@ Diags::~Diags()
//
// This routine takes an optional <debug_tag>, which is printed in
// parentheses if its value is not NULL. It takes a <diags_level>,
-// which is converted to a prefix string, unless <prefix> is set to
-// a non-NULL value (in which case <prefix> is used in preference.
+// which is converted to a prefix string.
// print_va takes an optional source location structure pointer <loc>,
// which can be NULL. If <loc> is not NULL, the source code location
// is converted to a string, and printed between angle brackets.
@@ -183,12 +182,11 @@ Diags::~Diags()
//////////////////////////////////////////////////////////////////////////////
void
-Diags::print_va(const char *debug_tag, DiagsLevel diags_level,
- const char *prefix, SrcLoc * loc,
+Diags::print_va(const char *debug_tag, DiagsLevel diags_level ,SrcLoc *loc,
const char *format_string, va_list ap)
{
struct timeval tp;
- const char *prefix_string, *s;
+ const char *s;
char *buffer, *d, timestamp_buf[48];
char format_buf[1024], format_buf_w_ts[1024], *end_of_format;
@@ -213,13 +211,11 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level,
pthread_t id = pthread_self();
end_of_format += snprintf(end_of_format, sizeof(format_buf), "{0x%" PRIx64 "} ", (uint64_t) id);
- ////////////////////////////////
- // start with the user prefix //
- ////////////////////////////////
-
- prefix_string = (prefix ? prefix : level_name(diags_level));
+ //////////////////////////////////////
+ // start with the diag level prefix //
+ //////////////////////////////////////
- for (s = prefix_string; *s; *end_of_format++ = *s++);
+ for (s = level_name(diags_level); *s; *end_of_format++ = *s++);
*end_of_format++ = ':';
*end_of_format++ = ' ';
@@ -271,7 +267,6 @@ Diags::print_va(const char *debug_tag, DiagsLevel diags_level,
*d++ = ']';
*d++ = ' ';
- //const char mgmt_str[] = "Manager ";
for (int k = 0; prefix_str[k]; k++)
*d++ = prefix_str[k];
for (s = format_buf; *s; *d++ = *s++);
@@ -509,60 +504,43 @@ Diags::dump(FILE * fp)
}
}
-
-
void
-DiagsDClosure::operator() (const char *tag, const char *format_string ...)
+Diags::log(const char *tag, DiagsLevel level,
+ const char *file, const char *func, const int line,
+ const char *format_string ...)
{
- va_list ap;
- va_start(ap, format_string);
- SrcLoc *lp = (diags->show_location ? &src_location : NULL);
- diags->log_va(tag, level, NULL, lp, format_string, ap);
- va_end(ap);
-}
+ if (! on(tag))
+ return;
-// location optionally printed
-void
-DiagsDClosure::operator() (const char *tag, int show_loc, const char *format_string ...)
-{
va_list ap;
va_start(ap, format_string);
- SrcLoc *lp = ((show_loc || diags->show_location) ? &src_location : NULL);
- diags->log_va(tag, level, NULL, lp, format_string, ap);
+ if (show_location) {
+ SrcLoc lp(file, func, line);
+ print_va(tag, level, &lp, format_string, ap);
+ } else {
+ print_va(tag, level, NULL, format_string, ap);
+ }
va_end(ap);
}
-
void
-DiagsEClosure::operator() (const char *format_string ...)
+Diags::error(DiagsLevel level,
+ const char *file, const char *func, const int line,
+ const char *format_string ...)
{
va_list ap;
va_start(ap, format_string);
- SrcLoc *lp = (diags->show_location ? &src_location : NULL);
- diags->print_va(NULL, level, NULL, lp, format_string, ap);
- va_end(ap);
- va_start(ap, format_string);
- if (DiagsLevel_IsTerminal(level)) {
- if (diags->cleanup_func)
- diags->cleanup_func();
- ink_fatal_va(1, (char *) format_string, ap);
+ if (show_location) {
+ SrcLoc lp(file, func, line);
+
+ print_va(NULL, level, &lp, format_string, ap);
+ } else {
+ print_va(NULL, level, NULL, format_string, ap);
}
- va_end(ap);
-}
-// location optionally printed
-void
-DiagsEClosure::operator() (int show_loc, const char *format_string ...)
-{
- va_list ap;
- va_start(ap, format_string);
- SrcLoc *lp = ((show_loc || diags->show_location) ? &src_location : NULL);
- diags->print_va(NULL, level, NULL, lp, format_string, ap);
- va_end(ap);
- va_start(ap, format_string);
if (DiagsLevel_IsTerminal(level)) {
- if (diags->cleanup_func)
- diags->cleanup_func();
+ if (cleanup_func)
+ cleanup_func();
ink_fatal_va(1, (char *) format_string, ap);
}
va_end(ap);
View
193 lib/ts/Diags.h
@@ -183,19 +183,19 @@ class Diags
const char *level_name(DiagsLevel dl);
- inkcoreapi void print_va(const char *tag, DiagsLevel dl, const char *prefix,
- SrcLoc * loc, const char *format_string, va_list ap);
+ inkcoreapi void print_va(const char *tag, DiagsLevel dl,
+ SrcLoc *loc, const char *format_string, va_list ap);
//////////////////////////////
// user printing interfaces //
//////////////////////////////
- void print(const char *tag, DiagsLevel dl, const char *prefix, SrcLoc * loc, const char *format_string, ...)
+ void print(const char *tag, DiagsLevel dl, SrcLoc * loc, const char *format_string, ...)
{
va_list ap;
va_start(ap, format_string);
- print_va(tag, dl, prefix, loc, format_string, ap);
+ print_va(tag, dl, loc, format_string, ap);
va_end(ap);
}
@@ -204,20 +204,20 @@ class Diags
// on the value of the enable flag, and the state of the debug tags. //
///////////////////////////////////////////////////////////////////////
- void log_va(const char *tag, DiagsLevel dl, const char *prefix, SrcLoc * loc, const char *format_string, va_list ap)
+ void log_va(const char *tag, DiagsLevel dl, SrcLoc * loc, const char *format_string, va_list ap)
{
if (!on(tag))
return;
- print_va(tag, dl, prefix, loc, format_string, ap);
+ print_va(tag, dl, loc, format_string, ap);
}
- void log(const char *tag, DiagsLevel dl, const char *prefix, SrcLoc * loc, const char *format_string, ...)
- {
- va_list ap;
- va_start(ap, format_string);
- log_va(tag, dl, prefix, loc, format_string, ap);
- va_end(ap);
- }
+ void log(const char *tag, DiagsLevel dl,
+ const char *file, const char *func, const int line,
+ const char *format_string, ...) TS_PRINTFLIKE(7, 8);
+
+ void error(DiagsLevel dl,
+ const char *file, const char *func, const int line,
+ const char *format_string, ...) TS_PRINTFLIKE(6, 7);
void dump(FILE * fp = stdout);
@@ -242,105 +242,6 @@ class Diags
}
};
-
-//////////////////////////////////////////////////////////////////////////////
-//
-// class DiagsBaseClosure
-// class DiagsDClosure
-// class DiagsEClosure
-//
-// The following classes are hacks. Their whole raison d'etre is to
-// make a macro that substitutes in __LINE__ and __FILE__ location
-// info in addition to the normal diagnostic logging arguments. But,
-// the lame cpp preprocessor doesn't let us represent and substitute
-// variable numbers of arguments, so we can't do this.
-//
-// Instead, a DiagsClosure is created at a point in the code (usually
-// within a macro that adds __LINE__ and __FILE__). DiagsClosure acts
-// as a closure, wrapping up this location context, and supporting
-// an operator() method that takes all the normal diagnostic log args.
-//
-// If the end, we can just say:
-//
-// Debug("http", "status = %d", status);
-//
-// This macro expands into an initialized creation of a DiagsClosure
-// that saves the location of the Debug macro & debug level, and
-// in effect invokes itself as a function of the original arguments:
-//
-// if (diags->on())
-// (*(new DiagsDClosure(diags,DL_Debug,_FILE_,_FUNC_,_LINE_)))
-// ("http", "status = %d", status)
-//
-// If you know a way to make this directly into a macro structure, and
-// bypass the DiagsClosure closures, be my guest...
-//
-// The DiagsDClosure and DiagsEClosure classes differ in that the
-// DiagsDClosure supports debug tags, and DiagsEClosure does not.
-//
-//////////////////////////////////////////////////////////////////////////////
-
-class DiagsBaseClosure
-{
-public:
- Diags *diags;
- DiagsLevel level;
- SrcLoc src_location;
-
- DiagsBaseClosure(Diags * d, DiagsLevel l, const char *_file, const char *_func, int _line)
- {
- diags = d;
- level = l;
- src_location.file = _file;
- src_location.func = _func;
- src_location.line = _line;
- src_location.valid = true;
- }
-
- ~DiagsBaseClosure()
- { }
-};
-
-
-class DiagsDClosure:public DiagsBaseClosure
-{
-public:
- DiagsDClosure(Diags * d, DiagsLevel l, const char *file, const char *func, int line):DiagsBaseClosure(d, l, file, func, line)
- { }
-
- ~DiagsDClosure()
- { }
-
- // default: no location printed
- void inkcoreapi operator() (const char *tag, const char *format_string ...)
- TS_PRINTFLIKE(3, 4);
-
- // location optionally printed
- void operator() (const char *tag, int show_loc, const char *format_string ...)
- TS_PRINTFLIKE(4, 5);
-};
-
-// debug closures support debug tags
-
-// error closures do not support debug tags
-class DiagsEClosure:public DiagsBaseClosure
-{
-public:
- DiagsEClosure(Diags * d, DiagsLevel l, const char *file, const char *func, int line):DiagsBaseClosure(d, l, file, func, line)
- { }
-
- ~DiagsEClosure()
- { }
-
- // default: no location printed
- void inkcoreapi operator() (const char *format_string ...)
- TS_PRINTFLIKE(2, 3);
-
- // location optionally printed
- void operator() (int show_loc, const char *format_string ...);
- TS_PRINTFLIKE(3, 4);
-};
-
//////////////////////////////////////////////////////////////////////////
// //
// Macros //
@@ -362,53 +263,33 @@ class DiagsEClosure:public DiagsBaseClosure
extern inkcoreapi Diags *diags;
-#define DTA(l) diags,l,__FILE__,__FUNCTION__,__LINE__
-
-void dummy_debug(const char * tag, const char *fmt ...) TS_PRINTFLIKE(2, 3);
-void dummy_debug(const char * tag, char *fmt ...) TS_PRINTFLIKE(2, 3);
-
-inline void
-dummy_debug(const char *, char *dummy_arg ...)
-{
- (void) dummy_arg;
-}
+#define DTA(l) l,__FILE__,__FUNCTION__,__LINE__
+void dummy_debug(const char * tag, const char *fmt, ...) TS_PRINTFLIKE(2, 3);
inline void
-dummy_debug(const char *, const char *dummy_arg ...)
+dummy_debug(const char *tag, const char *fmt, ...)
{
- (void) dummy_arg;
+ (void)tag;
+ (void)fmt;
}
-#if TS_USE_DIAGS
-#define Diag if (diags->on()) DiagsDClosure(DTA(DL_Diag)) /*args */
-#define Debug if (diags->on()) DiagsDClosure(DTA(DL_Debug)) /*args */
-#define DebugOn DiagsDClosure(DTA(DL_Debug)) /*args */
-#else
-#define Diag if (0) dummy_debug
-#define Debug if (0) dummy_debug
-#define DebugOn if (0) dummy_debug
-#endif
+#define Diag(tag, fmt, ...) diags->log(tag, DTA(DL_Diag), fmt, ##__VA_ARGS__)
+#define Debug(tag, fmt, ...) diags->log(tag, DTA(DL_Debug), fmt, ##__VA_ARGS__)
+#define DebugOn(tag, fmt, ...) diags->log(tag, DTA(DL_Debug), fmt, ##__VA_ARGS__)
-#define Status DiagsEClosure(DTA(DL_Status)) /*(args...) */
-#define Note DiagsEClosure(DTA(DL_Note)) /*(args...) */
-#define Warning DiagsEClosure(DTA(DL_Warning)) /*(args...) */
-#define Error DiagsEClosure(DTA(DL_Error)) /*(args...) */
-#define Fatal DiagsEClosure(DTA(DL_Fatal)) /*(args...) */
-#define Alert DiagsEClosure(DTA(DL_Alert)) /*(args...) */
-#define Emergency DiagsEClosure(DTA(DL_Emergency)) /*(args...) */
+#define Status(fmt, ...) diags->error(DTA(DL_Status), fmt, ##__VA_ARGS__)
+#define Note(fmt, ...) diags->error(DTA(DL_Note), fmt, ##__VA_ARGS__)
+#define Warning(fmt, ...) diags->error(DTA(DL_Warning), fmt, ##__VA_ARGS__)
+#define Error(fmt, ...) diags->error(DTA(DL_Error), fmt, ##__VA_ARGS__)
+#define Fatal(fmt, ...) diags->error(DTA(DL_Fatal), fmt, ##__VA_ARGS__)
+#define Alert(fmt, ...) diags->error(DTA(DL_Alert), fmt, ##__VA_ARGS__)
+#define Emergency(fmt, ...) diags->error(DTA(DL_Emergency), fmt, ##__VA_ARGS__)
-#if TS_USE_DIAGS
#define is_debug_tag_set(_t) diags->on(_t,DiagsTagType_Debug)
#define is_action_tag_set(_t) diags->on(_t,DiagsTagType_Action)
#define debug_tag_assert(_t,_a) (is_debug_tag_set(_t) ? (ink_release_assert(_a), 0) : 0)
#define action_tag_assert(_t,_a) (is_action_tag_set(_t) ? (ink_release_assert(_a), 0) : 0)
#define is_diags_on(_t) diags->on(_t)
-#else
-#define is_debug_tag_set(_t) 0
-#define is_action_tag_set(_t) 0
-#define debug_tag_assert(_t,_a) /**/
-#define action_tag_assert(_t,_a) /**/
-#endif
-#define stat_debug_assert(_tst) (void)((_tst) || (Warning(#_tst), debug_tag_assert("stat_check",! #_tst), 0))
+
#else // TS_USE_DIAGS
class Diags
@@ -436,19 +317,23 @@ extern inkcoreapi Diags *diags;
#define Alert ink_error
#define Emergency ink_fatal_die
+void dummy_debug(const char * tag, const char *fmt ...) TS_PRINTFLIKE(2, 3);
inline void
-dummy_debug(char *dummy_arg ...)
+dummy_debug(const char *tag, const char *fmt, ...)
{
- (void) dummy_arg;
+ (void)tag;
+ (void)fmt;
}
-#define Diag if (0) dummy_debug
-#define Debug if (0) dummy_debug
-#define DebugOn if (0) dummy_debug
+#define Diag(tag, fmt, ...) if (0) dummy_debug(tag, fmt, ##__VA_ARGS__)
+#define Debug(tag, fmt, ...) if (0) dummy_debug(tag, fmt, ##__VA_ARGS__)
+#define DebugOn(tag, fmt, ...) if (0) dummy_debug(tag, fmt, ##__VA_ARGS__)
+
#define is_debug_tag_set(_t) 0
#define is_action_tag_set(_t) 0
#define debug_tag_assert(_t,_a) /**/
-#define action_tag_assert(_t,_a)
+#define action_tag_assert(_t,_a) /**/
#define is_diags_on(_t) 0
+
#endif // TS_USE_DIAGS
#endif /*_Diags_h_*/
View
2 mgmt/api/CoreAPI.cc
@@ -133,7 +133,7 @@ Diags(TSDiagsT mode, const char *fmt, va_list ap)
}
if (diags_init) { // check that diags is initialized
- diags->print_va("TSMgmtAPI", level, NULL, NULL, fmt, ap);
+ diags->print_va("TSMgmtAPI", level, NULL, fmt, ap);
va_end(ap);
}
}
View
20 mgmt/utils/MgmtUtils.cc
@@ -325,7 +325,7 @@ mgmt_log(FILE * log, const char *message_format, ...)
#if defined(LOCAL_MANAGER) || defined(PROCESS_MANAGER)
if (diags_init) {
- diags->print_va(NULL, DL_Note, "NOTE", NULL, message_format, ap);
+ diags->print_va(NULL, DL_Note, NULL, message_format, ap);
} else {
#endif
@@ -355,7 +355,7 @@ mgmt_log(const char *message_format, ...)
va_start(ap, message_format);
#if defined(LOCAL_MANAGER) || defined(PROCESS_MANAGER)
if (diags_init) {
- diags->print_va(NULL, DL_Note, "NOTE", NULL, message_format, ap);
+ diags->print_va(NULL, DL_Note, NULL, message_format, ap);
} else {
#endif
@@ -392,9 +392,9 @@ mgmt_elog(FILE * log, const char *message_format, ...)
#if defined(LOCAL_MANAGER) || defined(PROCESS_MANAGER)
if (diags_init) {
int lerrno = errno;
- diags->print_va(NULL, DL_Error, "ERROR", NULL, message_format, ap);
+ diags->print_va(NULL, DL_Error, NULL, message_format, ap);
snprintf(message, sizeof(message), " (last system error %d: %s)\n", lerrno, strerror(lerrno));
- diags->print(NULL, DL_Error, "ERROR", NULL, message);
+ diags->print(NULL, DL_Error, NULL, message);
} else {
#endif
if (use_syslog) {
@@ -431,9 +431,9 @@ mgmt_elog(const char *message_format, ...)
#if defined(LOCAL_MANAGER) || defined(PROCESS_MANAGER)
if (diags_init) {
int lerrno = errno;
- diags->print_va(NULL, DL_Error, "ERROR", NULL, message_format, ap);
+ diags->print_va(NULL, DL_Error, NULL, message_format, ap);
snprintf(message, sizeof(message), " (last system error %d: %s)\n", lerrno, strerror(lerrno));
- diags->print(NULL, DL_Error, "ERROR", NULL, message);
+ diags->print(NULL, DL_Error, NULL, message);
} else {
#endif
@@ -476,9 +476,9 @@ mgmt_fatal(FILE * log, const char *message_format, ...)
#if defined(LOCAL_MANAGER) || defined(PROCESS_MANAGER)
if (diags_init) {
int lerrno = errno;
- diags->print_va(NULL, DL_Fatal, "FATAL", NULL, message_format, ap);
+ diags->print_va(NULL, DL_Fatal, NULL, message_format, ap);
snprintf(message, sizeof(message), " (last system error %d: %s)\n", lerrno, strerror(lerrno));
- diags->print(NULL, DL_Fatal, "FATAL", NULL, message);
+ diags->print(NULL, DL_Fatal, NULL, message);
} else {
#endif
@@ -523,9 +523,9 @@ mgmt_fatal(const char *message_format, ...)
#if defined(LOCAL_MANAGER) || defined(PROCESS_MANAGER)
if (diags_init) {
int lerrno = errno;
- diags->print_va(NULL, DL_Fatal, "FATAL", NULL, message_format, ap);
+ diags->print_va(NULL, DL_Fatal, NULL, message_format, ap);
snprintf(message, sizeof(message), " (last system error %d: %s)\n", lerrno, strerror(lerrno));
- diags->print(NULL, DL_Fatal, "FATAL", NULL, message);
+ diags->print(NULL, DL_Fatal, NULL, message);
} else {
#endif
View
10 proxy/DiagsConfig.cc
@@ -121,7 +121,7 @@ DiagsConfig::reconfigure_diags()
ats_free(p);
} else {
SrcLoc loc(__FILE__, __FUNCTION__, __LINE__);
- diags->print(NULL, DL_Error, NULL, &loc, "can't find config variable '%s'\n", record_name);
+ diags->print(NULL, DL_Error, &loc, "can't find config variable '%s'\n", record_name);
}
}
@@ -140,7 +140,7 @@ DiagsConfig::reconfigure_diags()
if (!all_found) {
SrcLoc loc(__FILE__, __FUNCTION__, __LINE__);
- diags->print(NULL, DL_Error, NULL, &loc, "couldn't fetch all proxy.config.diags values");
+ diags->print(NULL, DL_Error, &loc, "couldn't fetch all proxy.config.diags values");
} else {
//////////////////////////////
// clear out old tag tables //
@@ -164,7 +164,7 @@ DiagsConfig::reconfigure_diags()
#else
memcpy(((void *) &diags->config), ((void *) &c), sizeof(DiagsConfigState));
#endif
- diags->print(NULL, DL_Note, NULL, NULL, "updated diags config");
+ diags->print(NULL, DL_Note, NULL, "updated diags config");
}
////////////////////////////////////
@@ -343,10 +343,10 @@ DiagsConfig::DiagsConfig(char *bdt, char *bat, bool use_records)
if (diags_log_fp == NULL) {
SrcLoc loc(__FILE__, __FUNCTION__, __LINE__);
- diags->print(NULL, DL_Warning, NULL, &loc,
+ diags->print(NULL, DL_Warning, &loc,
"couldn't open diags log file '%s', " "will not log to this file", diags_logpath);
}
- diags->print(NULL, DL_Status, "STATUS", NULL, "opened %s", diags_logpath);
+ diags->print(NULL, DL_Status, NULL, "opened %s", diags_logpath);
register_diags_callbacks();
View
2 proxy/Error.cc
@@ -58,7 +58,7 @@ ErrorClass::raise(va_list ap, const char *prefix)
NOWARN_UNUSED(prefix);
SrcLoc loc;
loc.set(filename, function_name, line_number);
- diags->print_va(NULL, DL_Fatal, NULL, &loc, format_string, ap);
+ diags->print_va(NULL, DL_Fatal, &loc, format_string, ap);
}
// Request Fatal
View
4 proxy/InkAPI.cc
@@ -386,7 +386,7 @@ TSError(const char *fmt, ...)
if (is_action_tag_set("deft") || is_action_tag_set("sdk_vbos_errors")) {
va_start(args, fmt);
- diags->print_va(NULL, DL_Error, NULL, NULL, fmt, args);
+ diags->print_va(NULL, DL_Error, NULL, fmt, args);
va_end(args);
}
va_start(args, fmt);
@@ -6823,7 +6823,7 @@ TSDebug(const char *tag, const char *format_str, ...)
va_list ap;
va_start(ap, format_str);
- diags->print_va(tag, DL_Diag, NULL, NULL, format_str, ap);
+ diags->print_va(tag, DL_Diag, NULL, format_str, ap);
va_end(ap);
}
}
View
1 proxy/UglyLogStubs.cc
@@ -133,7 +133,6 @@ LogCollationClientSM::LogCollationClientSM(LogHost * log_host):
m_abort_buffer(NULL),
m_buffer_send_list(NULL), m_buffer_in_iocore(NULL), m_flow(LOG_COLL_FLOW_ALLOW), m_log_host(log_host), m_id(0)
{
- Debug("log-coll", "[%d]client::constructor", m_id);
}
LogCollationClientSM::~LogCollationClientSM()

0 comments on commit 5ad005c

Please sign in to comment.
Something went wrong with that request. Please try again.