Re: [Log4cplus-devel] log4cplus 1.0.4-RC10 released

[Václav H. <v.h...@sh...>]

On Mon, 19 Jul 2010 13:55:40 +0200, Hannah Schroeter wrote:
> Hi!
> 
> On Sun, Jul 18, 2010 at 12:22:12PM +0200, Václav Haisman wrote:
>>I have removed the %X{N} PatternLayout format specifier introduced in
>>previous RC because conflicts with PatternLayout format specifier for
MDC,
>>which has been added to trunk in the repository.
> 
> I see the point of freeing %X for MDC. However, do I see things right
> that using a property for an NDC depth limit means that things are more
> "global"?
> 
> With a format parameter (even if it be an optional parameter to %x, the
> *existing* NDC converter), you could format the NDC multiple times in a
> single format, using *different* limits (e.g. using a tight limit
> relatively early in the log line, and using no limit late in the line,
> so people reading the log could see the limited NDC at the first glance,
> but still the complete NDC is not completely lost). But if I understand
> the property thing right, this wouldn't be possible now, as the property
> influences NDC formatting in a more "global" way now (at least inside
> eacy patternlayout).
I think that printing the NDC more than once for each event is not that
useful. But I guess that in special circumstances it can be so. The problem
with extra precision specification for the %x format specifier is that it
makes log4cplus diverge from log4j. E.g. the format specified by string
"%x{hi there}" would print "context1 context2{hi there}" from log4j and
log4cplus prior to 1.0.4 but it would print "context1 context2" from
log4cplus 1.0.4 (because "hi there" is not a number and it would be
interpreted as "0" which means unlimited NDC depth).

I think that I understand your desire to have the feature in log4cplus
repository instead of only as your local patch. May I suggest that you
instead copy the PatternLayout code into a new class, do the modification
and register the new class as layout with the LayoutFactoryRegistry?
Looking at the code it seems you will have to copy most of
patternlayout.cxx as some of the classes are not part of public interface.

> 
>>[...]
> 
>>Let's hope no more bugs will crop up and that this will really be the
last
>>release candidate for log4cplus 1.0.4. :)
> 
> I see a long standing coding bug in src/configurator.cxx, in line 729 of
> PRODUCTION_1_0_x r1434. Here, the lstat() return status isn't checked,
> which might cause unexpected function results if the filename checked
> denotes a broken symbolic link.
Thank you, good catch. I have fixed this in revision 1437.


-- 
VH