diff options
| -rw-r--r-- | ChangeLog | 31 | ||||
| -rw-r--r-- | ChangeLogs/ChangeLog-02a | 31 | ||||
| -rw-r--r-- | ChangeLogs/ChangeLog-03a | 31 | ||||
| -rw-r--r-- | ace/Log_Msg.cpp | 33 | ||||
| -rw-r--r-- | ace/Log_Msg.h | 50 |
5 files changed, 154 insertions, 22 deletions
diff --git a/ChangeLog b/ChangeLog index 79f0f729c1a..6d267c0e968 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,34 @@ +Thu Sep 28 22:13:04 2000 Irfan Pyarali <irfan@cs.wustl.edu> + + * ace/Log_Msg: Macros are evil! All the ACE debugging macros were + evaluating the user arguments twice, once in + log_priority_enabled() and the other in log(). The problem was + that set() was making a deep copy of the filename. Hence, this + change: + + Wed May 3 11:43:05 2000 Douglas C. Schmidt <schmidt@ace.cs.wustl.edu> + + was made to prevent the deep copy which was unnecessary when the + message was not actually logged. However, this resulted in the + macro evaluating the user arguments twice. Previously, a + statement like this: + + ACE_DEBUG ((LM_DEBUG, + "timeout occured, iterations left %d\n", + --iterations)); + + <iterations> got reduced by one - now it got reduced by two ;-) + + The solution was to make a shallow copy of the filename in + conditional_set(). Then in log(), if the log priority is + correct, make a deep copy and then continue will log(). The + macros were changed to call conditional_set() instead of set(). + + Also, changed ACE_RETURN to specify all the parameters, + including <restart>, <callback>, and <stream>. Otherwise, the + default parameters of set() will end up losing these parameters + set by the user. + Thu Sep 28 15:49:00 2000 Douglas C. Schmidt <schmidt@ace.cs.wustl.edu> * ace/Stream.h (class ACE_Stream): Updated the documentation to clarify diff --git a/ChangeLogs/ChangeLog-02a b/ChangeLogs/ChangeLog-02a index 79f0f729c1a..6d267c0e968 100644 --- a/ChangeLogs/ChangeLog-02a +++ b/ChangeLogs/ChangeLog-02a @@ -1,3 +1,34 @@ +Thu Sep 28 22:13:04 2000 Irfan Pyarali <irfan@cs.wustl.edu> + + * ace/Log_Msg: Macros are evil! All the ACE debugging macros were + evaluating the user arguments twice, once in + log_priority_enabled() and the other in log(). The problem was + that set() was making a deep copy of the filename. Hence, this + change: + + Wed May 3 11:43:05 2000 Douglas C. Schmidt <schmidt@ace.cs.wustl.edu> + + was made to prevent the deep copy which was unnecessary when the + message was not actually logged. However, this resulted in the + macro evaluating the user arguments twice. Previously, a + statement like this: + + ACE_DEBUG ((LM_DEBUG, + "timeout occured, iterations left %d\n", + --iterations)); + + <iterations> got reduced by one - now it got reduced by two ;-) + + The solution was to make a shallow copy of the filename in + conditional_set(). Then in log(), if the log priority is + correct, make a deep copy and then continue will log(). The + macros were changed to call conditional_set() instead of set(). + + Also, changed ACE_RETURN to specify all the parameters, + including <restart>, <callback>, and <stream>. Otherwise, the + default parameters of set() will end up losing these parameters + set by the user. + Thu Sep 28 15:49:00 2000 Douglas C. Schmidt <schmidt@ace.cs.wustl.edu> * ace/Stream.h (class ACE_Stream): Updated the documentation to clarify diff --git a/ChangeLogs/ChangeLog-03a b/ChangeLogs/ChangeLog-03a index 79f0f729c1a..6d267c0e968 100644 --- a/ChangeLogs/ChangeLog-03a +++ b/ChangeLogs/ChangeLog-03a @@ -1,3 +1,34 @@ +Thu Sep 28 22:13:04 2000 Irfan Pyarali <irfan@cs.wustl.edu> + + * ace/Log_Msg: Macros are evil! All the ACE debugging macros were + evaluating the user arguments twice, once in + log_priority_enabled() and the other in log(). The problem was + that set() was making a deep copy of the filename. Hence, this + change: + + Wed May 3 11:43:05 2000 Douglas C. Schmidt <schmidt@ace.cs.wustl.edu> + + was made to prevent the deep copy which was unnecessary when the + message was not actually logged. However, this resulted in the + macro evaluating the user arguments twice. Previously, a + statement like this: + + ACE_DEBUG ((LM_DEBUG, + "timeout occured, iterations left %d\n", + --iterations)); + + <iterations> got reduced by one - now it got reduced by two ;-) + + The solution was to make a shallow copy of the filename in + conditional_set(). Then in log(), if the log priority is + correct, make a deep copy and then continue will log(). The + macros were changed to call conditional_set() instead of set(). + + Also, changed ACE_RETURN to specify all the parameters, + including <restart>, <callback>, and <stream>. Otherwise, the + default parameters of set() will end up losing these parameters + set by the user. + Thu Sep 28 15:49:00 2000 Douglas C. Schmidt <schmidt@ace.cs.wustl.edu> * ace/Stream.h (class ACE_Stream): Updated the documentation to clarify diff --git a/ace/Log_Msg.cpp b/ace/Log_Msg.cpp index bee6de900ff..c68079e96a4 100644 --- a/ace/Log_Msg.cpp +++ b/ace/Log_Msg.cpp @@ -523,6 +523,8 @@ ACE_Log_Msg::ACE_Log_Msg (void) ACE_Log_Msg::close, ACE_Log_Msg::sync_hook, ACE_Log_Msg::thr_desc_hook); + + this->conditional_values_.is_set_ = 0; } ACE_Log_Msg::~ACE_Log_Msg (void) @@ -762,12 +764,28 @@ ACE_Log_Msg::log (const ACE_TCHAR *format_str, #endif /* ! (defined(__BORLANDC__) && __BORLANDC__ >= 0x0530) */ typedef void (*PTF)(...); + // Check if there were any conditional values set. + int conditional_values = this->conditional_values_.is_set_; + + // Reset conditional values. + this->conditional_values_.is_set_ = 0; + // Only print the message if <priority_mask_> hasn't been reset to // exclude this logging priority. - if (this->log_priority_enabled (log_priority) == 0) return 0; + // If conditional values were set and the log priority is correct, + // then the values are actually set. + if (conditional_values) + this->set (this->conditional_values_.file_, + this->conditional_values_.line_, + this->conditional_values_.op_status_, + this->conditional_values_.errnum_, + this->restart (), + this->msg_ostream (), + this->msg_callback ()); + // Logging is a benign activity, so don't inadvertently smash errno. ACE_Errno_Guard guard (errno); @@ -1497,6 +1515,19 @@ ACE_Log_Msg::set (const ACE_TCHAR *filename, } void +ACE_Log_Msg::conditional_set (const ACE_TCHAR *filename, + int line, + int status, + int err) +{ + this->conditional_values_.is_set_ = 1; + this->conditional_values_.file_ = filename; + this->conditional_values_.line_ = line; + this->conditional_values_.op_status_ = status; + this->conditional_values_.errnum_ = err; +} + +void ACE_Log_Msg::dump (void) const { ACE_TRACE ("ACE_Log_Msg::dump"); diff --git a/ace/Log_Msg.h b/ace/Log_Msg.h index 4f0696021d6..bff510b68e2 100644 --- a/ace/Log_Msg.h +++ b/ace/Log_Msg.h @@ -52,48 +52,37 @@ do { \ int __ace_error = ACE_OS::last_error (); \ ACE_Log_Msg *ace___ = ACE_Log_Msg::instance (); \ - if (ace___->log_priority_enabled X != 0) { \ - ace___->set (ACE_TEXT_CHAR_TO_TCHAR(__FILE__), __LINE__, 0, __ace_error, ace___->restart (), \ - ace___->msg_ostream (), ace___->msg_callback ()); \ - ace___->log_hexdump X; \ - } \ + ace___->conditional_set (ACE_TEXT_CHAR_TO_TCHAR(__FILE__), __LINE__, 0, __ace_error); \ + ace___->log_hexdump X; \ } while (0) #define ACE_RETURN(Y) \ do { \ int __ace_error = ACE_OS::last_error (); \ - ACE_Log_Msg::instance ()->set (ACE_TEXT_CHAR_TO_TCHAR (__FILE__), __LINE__, Y, __ace_error); \ + ACE_Log_Msg::instance ()->set (ACE_TEXT_CHAR_TO_TCHAR (__FILE__), __LINE__, Y, __ace_error, ace___->restart (), \ + ace___->msg_ostream (), ace___->msg_callback ()); \ return Y; \ } while (0) #define ACE_ERROR_RETURN(X, Y) \ do { \ int __ace_error = ACE_OS::last_error (); \ ACE_Log_Msg *ace___ = ACE_Log_Msg::instance (); \ - if (ace___->log_priority_enabled X != 0) { \ - ace___->set (ACE_TEXT_CHAR_TO_TCHAR (__FILE__), __LINE__, Y, __ace_error, ace___->restart (), \ - ace___->msg_ostream (), ace___->msg_callback ()); \ - ace___->log X; \ - } \ + ace___->conditional_set (ACE_TEXT_CHAR_TO_TCHAR (__FILE__), __LINE__, Y, __ace_error); \ + ace___->log X; \ return Y; \ } while (0) #define ACE_ERROR(X) \ do { \ int __ace_error = ACE_OS::last_error (); \ ACE_Log_Msg *ace___ = ACE_Log_Msg::instance (); \ - if (ace___->log_priority_enabled X != 0) { \ - ace___->set (ACE_TEXT_CHAR_TO_TCHAR (__FILE__), __LINE__, -1, __ace_error, ace___->restart (), \ - ace___->msg_ostream (), ace___->msg_callback ()); \ - ace___->log X; \ - } \ + ace___->conditional_set (ACE_TEXT_CHAR_TO_TCHAR (__FILE__), __LINE__, -1, __ace_error); \ + ace___->log X; \ } while (0) #define ACE_DEBUG(X) \ do { \ int __ace_error = ACE_OS::last_error (); \ ACE_Log_Msg *ace___ = ACE_Log_Msg::instance (); \ - if (ace___->log_priority_enabled X != 0) { \ - ace___->set (ACE_TEXT_CHAR_TO_TCHAR(__FILE__), __LINE__, 0, __ace_error, ace___->restart (), \ - ace___->msg_ostream (), ace___->msg_callback ()); \ - ace___->log X; \ - } \ + ace___->conditional_set (ACE_TEXT_CHAR_TO_TCHAR(__FILE__), __LINE__, 0, __ace_error); \ + ace___->log X; \ } while (0) #define ACE_ERROR_INIT(VALUE, FLAGS) \ do { \ @@ -491,6 +480,13 @@ public: // restart flag, ostream, and the callback object. This combines // all the other set methods into a single method. + void conditional_set (const ACE_TCHAR *file, + int line, + int op_status, + int errnum); + // These values are only actually set if the requested priority is + // enabled. + ssize_t log (ACE_Log_Priority priority, const ACE_TCHAR *format, ...); // Format a message to the thread-safe ACE logging mechanism. Valid // options (prefixed by '%', as in printf format strings) include: @@ -625,6 +621,18 @@ private: static u_long default_priority_mask_; // Priority mask to use for each new instance + // Anonymous struct since there will only be one instance. This + // struct keeps information stored away in case we actually end up + // calling log() if the log priority is correct. + struct + { + int is_set_; + const ACE_TCHAR *file_; + int line_; + int op_status_; + int errnum_; + } conditional_values_; + #if defined (ACE_MT_SAFE) && (ACE_MT_SAFE != 0) static int key_created_; # if defined (ACE_HAS_THREAD_SPECIFIC_STORAGE) || \ |