-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add deregistry function for LogAppender. #17
Conversation
The same function needs to be added to the other backends. |
Add deregistry to other backends as well.
I don't know why build fails, Jenkins shows nothing about the failure. I thought it was because of multiple commits, so squashed them but didn't work either. |
The Jenkins build output contains:
|
Thanks for your patient guidance :) |
@@ -97,6 +97,10 @@ void register_appender(LogAppender* appender) | |||
rosconsole_glog_appender = appender; | |||
} | |||
|
|||
void deregister_appender(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the same style of the file: putting the {
in a new line.
Same below.
const log4cxx::LoggerPtr& logger = log4cxx::Logger::getLogger(ROSCONSOLE_ROOT_LOGGER_NAME); | ||
logger->removeAppender(g_log4cxx_appender); | ||
delete g_log4cxx_appender; | ||
g_log4cxx_appender = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please follow the same style of the file: 2 space indentation (no tabs).
Same below.
@@ -97,6 +97,10 @@ void register_appender(LogAppender* appender) | |||
rosconsole_glog_appender = appender; | |||
} | |||
|
|||
void deregister_appender(){ | |||
rosconsole_glog_appender = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use 0
here for consistency - that is the default value further up in the same file and in the other implementations.
Thank you for adding this feature. |
Resolution of memory leak issue in ROSOutAppender requires deregistration of
g_rosout_appender
. This PR implements that functionality.