rm5248 commented on code in PR #130:
URL: https://github.com/apache/logging-log4cxx/pull/130#discussion_r972542642
##########
src/main/cpp/loggingevent.cpp:
##########
@@ -297,40 +302,62 @@ LoggingEvent::KeySet LoggingEvent::getPropertyKeySet()
const
}
-const LogString LoggingEvent::getCurrentThreadName()
+const LogString& LoggingEvent::getCurrentThreadName()
{
-#if APR_HAS_THREADS
- LOG4CXX_THREAD_LOCAL LogString thread_name;
+#if defined(_WIN32)
+ using ThreadIdType = DWORD;
+ ThreadIdType threadId = GetCurrentThreadId();
+#elif APR_HAS_THREADS
+ using ThreadIdType = apr_os_thread_t;
+ ThreadIdType threadId = apr_os_thread_current();
+#else
+ using ThreadIdType = int;
+ ThreadIdType threadId = 0;
+#endif
- if ( thread_name.size() )
+#if defined(__BORLANDC__)
Review Comment:
It seems that `thread_local` is still not supported even in the latest
compiler:
https://docwiki.embarcadero.com/RADStudio/Alexandria/en/Modern_C%2B%2B_Language_Features_Compliance_Status
This is rather unfortunate, because that is very useful for things like this.
It seems like we should do the following:
- Remove the Borland-specific code that was added here. Changing the log
event to hold a reference instead of a copy is a good change and should be kept
- Have a discussion on the mailing as to the level of C++ support and the
compilers supported. We should be able to support the Borland compilers
without too much trouble as long as we keep away from certain features.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
