Re: [Log4cplus-devel] Rest of the patches (was Re: log4cplus 1.0.4-RC8 released)

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

On Wed, 2 Jun 2010 16:06:21 +0200, Hannah Schroeter wrote:
> Hi!
> 
> On Wed, Jun 02, 2010 at 12:01:33AM +0200, Václav Haisman wrote:
>>[...]
> 
>>Yes, I will consider any patches you send.
> 
>>[...]
> 
>>The best for me is if you base the patches on the HEAD revision of e.g.
>>PRODUCTION_1_0_x branch. But it does not matter much, I can deal with
>>patches
>>against release source as well.
> 
> No problem, rebasing them wasn't difficult.
> 
>>> Based on your feedback, I could send you the other patches which would
>>> encompass another fix for a possible problem we noticed here in our
>>> company, and a few feature additions we've come to depend upon here.
> 
>>Please send the patches. I will consider each one of them separately.
Use
>>either the mailing list or the patches tracker on SF, whichever is more
>>comfortable.
> 
> Sending them here.
> 
> Little explanations.
> 
> 0001 seems to fix a bug for the syslog appender in some cases (if either
> log4cplus uses wstrings and thus LOG4CPLUS_TSTRING_TO_STRING returns a
> temporary, or the identifier is derived from a property, so the
> identifier is always a temporary). At least glibc always stores the
> char* you pass it, instead of copying it by itself.
Applied with minor changes.

> 
> 0002 is a feature addition we needed in house. log4cplus already
> supports logging of the thread ID returned by pthread_self, however, for
> us that wasn't as useful as the gettid() id, as they're different and ps
> shows only the latter (Linux uses 1:1 threading).
I am rejecting this patch. I am not against the idea of this but the
implementation needs some polishing first.

The thread ID needs to be part of the InternalLoggingEvent instance
because the events can be passed around between threads and the thread that
is formatting the output does not have to be the same thread in which the
event has occurred. The thread ID is also not passed through SocketAppender
but you can ignore that, it is a similar situation as with %h/%H format
specifier for host name which does not make it through either.

I think that %T should expand to the same string as %t does when the given
OS does not have any meaningful gettid()-like function.

> 
> 0003 is a feature addition analogous to acces the behavior of
> openlog itself, namely being able to pass the NULL pointer to openlog to
> access a default.
Applied with minor changes.

> 
> 0004 makes the syslog *facility* configurable, instead of only being
> able to use the default (LOG_USER).
Applied with minor changes.

> 
> 0005 makes it possible to limit the depth of the NDC in the pattern
> layout.
I have not had the time to apply the patch, yet, but I think is it looks
ok and I will accept it.

I am going to work on the thread ID patch myself, using yours as a
starting point, but I am going release RC9 first with only the rest of the
patches applied.

Thanks.

--
VH