ams-tschoening commented on a change in pull request #70:
URL: https://github.com/apache/logging-log4cxx/pull/70#discussion_r706822667



##########
File path: src/main/cpp/domconfigurator.cpp
##########
@@ -1035,6 +1037,19 @@ void DOMConfigurator::parse(
                MessageBufferUseStaticStream();
        }
 
+       LogString threadSignalValue = subst(getAttribute(utf8Decoder, element, 
THREAD_TYPE_ATTR));
+       if( !threadSignalValue.empty() && threadSignalValue != NuLL ){

Review comment:
       While it's not strictly related to your PR, `NuLL` reads a bit 
surprising to me. How about changig that to something like `[STR_]NULL[_STR]` 
instead?

##########
File path: src/main/cpp/domconfigurator.cpp
##########
@@ -113,6 +114,7 @@ IMPLEMENT_LOG4CXX_OBJECT(DOMConfigurator)
 #define STRINGSTREAM_ATTR "stringstream"
 #define CONFIG_DEBUG_ATTR "configDebug"
 #define INTERNAL_DEBUG_ATTR "debug"
+#define THREAD_TYPE_ATTR "threadConfiguration"

Review comment:
       Opinion: `THREAD_CONFIG_ATTR` seems a more reasonable name to me. `TYPE` 
reads like different types of threads, daemon threads vs. foreground threads 
vs. ..., while it's actually about their config.

##########
File path: src/site/markdown/threading.md
##########
@@ -88,6 +93,27 @@ call, as there is an internal mutex that is locked and 
unlocked in order to ensu
 only one thread can be started at a time.  Failure to do this may lead to 
deadlock.
 The ThreadUtility::configure method handles this automatically.
 
+### Configuring Thread {#configuring}
+
+To tell Log4cxx what to do by default when starting a new thread, the enum
+[ThreadConfigurationType](@ref log4cxx.helpers.ThreadConfigurationType) may be
+used to configure the library appropriately.  By default, all signals on POSIX
+systems will be blocked to ensure that other threads do not get signals.
+
+To change this default, a simple change to your configuration files may be 
done.
+
+Example with XML configuration:
+```
+<log4j:configuration threadConfiguration="BlockSignalsOnly">

Review comment:
       Opinion: As `BlockSignalsOnly` is the default already and you talk about 
changing that, how about using `NameThreadOnly` or some other option as example 
instead? That's what I expected after reading your text. As you have two 
examples, you could as well use two different keywords.




-- 
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


Reply via email to