I added a createDefaultLayout() method to PatternLayout, but I don't think
I addressed the header/footer concern. I did, however, replace all those
calls to createLayout() with a bunch of null values as part of the
migration to typed parameters in factory methods.


On 20 May 2014 22:55, Gary Gregory <[email protected]> wrote:

> Plenty of tests pass null for the config to PatternLayout.
>
> If you look at readers of the config ivar, NPE's can be happen, in theory,
> in getFooter() and getHeader(). createPatternParser() handles a null config
> with a special case.
>
> If look at PatternLayoutTest, all tests make sure to pass a valid config
> object to create a layout.
>
> Either:
> - we should document null as OK in special cases.
> - and/or: guard against null in getFooter/Header()
> - or: fix createLayout to use a default config.
>
> I naively tried to use a default config in createLayout:
>
>         return new PatternLayout(
>                 config == null ? ((LoggerContext)
> LogManager.getContext(false)).getConfiguration() : config, replace,
>                 pattern == null ? DEFAULT_CONVERSION_PATTERN : pattern,
> charset, alwaysWriteExceptions,
>                 noConsoleNoAnsi, header, footer);
>
> Boom! Infinite loop:
>
>         at
> org.apache.logging.log4j.core.layout.PatternLayout.createLayout(PatternLayout.java:286)
>         at
> org.apache.logging.log4j.core.config.DefaultConfiguration.<init>(DefaultConfiguration.java:55)
>         at
> org.apache.logging.log4j.core.LoggerContext.<init>(LoggerContext.java:71)
>         at
> org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.locateContext(ClassLoaderContextSelector.java:218)
>         at
> org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:113)
>         at
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:121)
>         at
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:36)
>         at
> org.apache.logging.log4j.LogManager.getContext(LogManager.java:164)
>         at
> org.apache.logging.log4j.core.layout.PatternLayout.createLayout(PatternLayout.java:286)
>         at
> org.apache.logging.log4j.core.config.DefaultConfiguration.<init>(DefaultConfiguration.java:55)
>         at
> org.apache.logging.log4j.core.LoggerContext.<init>(LoggerContext.java:71)
>         at
> org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.locateContext(ClassLoaderContextSelector.java:218)
>         at
> org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:113)
>         at
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:121)
>         at
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:36)
>         at
> org.apache.logging.log4j.LogManager.getContext(LogManager.java:164)
>         at
> org.apache.logging.log4j.core.layout.PatternLayout.createLayout(PatternLayout.java:286)
>         at
> org.apache.logging.log4j.core.config.DefaultConfiguration.<init>(DefaultConfiguration.java:55)
>         at
> org.apache.logging.log4j.core.LoggerContext.<init>(LoggerContext.java:71)
>         at
> org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.locateContext(ClassLoaderContextSelector.java:218)
>         at
> org.apache.logging.log4j.core.selector.ClassLoaderContextSelector.getContext(ClassLoaderContextSelector.java:113)
>         at
> org.apache.logging.log4j.core.impl.Log4jContextFactory.getContext(Log4jContextFactory.java:121)
>
> Same kinda thing with:
>
> ((LoggerContext) LogManager.getContext()).getConfiguration()
>
> Thoughts?
>
> Thank you,
> Gary
>
> --
> E-Mail: [email protected] | [email protected]
> Java Persistence with Hibernate, Second 
> Edition<http://www.manning.com/bauer3/>
> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> Spring Batch in Action <http://www.manning.com/templier/>
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
>



-- 
Matt Sicker <[email protected]>

Reply via email to