LOG4CXX_ERROR/WARN/INFO and related macro changes, revision: 308702

[Sean Hermany <sh...@factset.com>]

Revision number 308702 changed the way the LOG4CXX_INFO...etc macros 
behave. This has broken a substantial portion of code for me, and I think 
the change was a "downgrade" of sorts.

For instance:
 #define LOG4CXX_WARN(logger, message) { \
-       if (logger->isWarnEnabled()) {\
-       ::log4cxx::StringBuffer oss; \
-       oss << message; \
-       logger->forcedLog(::log4cxx::Level::WARN, oss.str(), __FILE__, 
__LINE__); }}
+        if (logger->isWarnEnabled()) {\
+            logger->forcedLog(::log4cxx::Level::WARN, message, 
LOG4CXX_LOCATION); }}

The net effect is that previously, you were able to do:

LOG4CXX_WARN(myLogger, "Dropped" << numPacketsDropped << " of " << 
totalNumPackets << " packets.");

whereas this no longer works. It is true that you could create a string 
buffer yourself outside of the logging call. However, then you pay the 
creation cost of the string buffer regardless of whether logging for that 
level is enabled or not. OK - one could wrap the whole thing in an 
if(logger->isWarnEnabled()) call as before. To that effect, I could just 
create new macros that do just what the old ones did.

What I'm getting at is that I think the old macros were much more useful, 
and it would be better to revert to them (keeping the change to use 
LOG4CXX_LOCATION of course). Changing back to the macro version that 
created its own string buffer would not break any existing code. On the 
other hand, if/when a .98 release does come it, this change WILL break any 
other .97 users who were using the macros as my codebase does.

I realize this revision is around 2.5 years old. We are only noticing the 
effects of it here because we have been using version .97 up until now. We 
only recently upgraded to the head revision of log4cxx during the process 
of moving our code to Visual Studio 2005(8).

Sean Hermany
Software Engineer
FactSet Research Systems Inc.
shermany@factset.com | 650.286.4942
http://www.factset.com

Re: LOG4CXX_ERROR/WARN/INFO and related macro changes, revision: 308702

[Curt Arnold <ca...@apache.org>]

The main problem with the old macros is that they were limited to  
only supporting one type of character string which would have  
required distinct macros for each character type.  The current macro  
definition does allow you to log all supported character types with  
one set of macros, but does lose the ability to throw in insertion  
operation since there is no mechanism for matching the string types  
used in the fragment with the appropriate string type.  Maybe there  
is a compromise where you could have an wrap an  
std::basic_string<logchar> in extended class that would do the  
transcoding from wchar_t, char*.  Let me thing about it and maybe I  
can come up with something that spans the gap.



On May 16, 2007, at 7:48 PM, Sean Hermany wrote:

> Revision number 308702 changed the way the LOG4CXX_INFO...etc macros
> behave. This has broken a substantial portion of code for me, and I  
> think
> the change was a "downgrade" of sorts.
>
> For instance:
>  #define LOG4CXX_WARN(logger, message) { \
> -       if (logger->isWarnEnabled()) {\
> -       ::log4cxx::StringBuffer oss; \
> -       oss << message; \
> -       logger->forcedLog(::log4cxx::Level::WARN, oss.str(), __FILE__,
> __LINE__); }}
> +        if (logger->isWarnEnabled()) {\
> +            logger->forcedLog(::log4cxx::Level::WARN, message,
> LOG4CXX_LOCATION); }}
>
> The net effect is that previously, you were able to do:
>
> LOG4CXX_WARN(myLogger, "Dropped" << numPacketsDropped << " of " <<
> totalNumPackets << " packets.");
>
> whereas this no longer works. It is true that you could create a  
> string
> buffer yourself outside of the logging call. However, then you pay the
> creation cost of the string buffer regardless of whether logging  
> for that
> level is enabled or not. OK - one could wrap the whole thing in an
> if(logger->isWarnEnabled()) call as before. To that effect, I could  
> just
> create new macros that do just what the old ones did.
>
> What I'm getting at is that I think the old macros were much more  
> useful,
> and it would be better to revert to them (keeping the change to use
> LOG4CXX_LOCATION of course). Changing back to the macro version that
> created its own string buffer would not break any existing code. On  
> the
> other hand, if/when a .98 release does come it, this change WILL  
> break any
> other .97 users who were using the macros as my codebase does.
>
> I realize this revision is around 2.5 years old. We are only  
> noticing the
> effects of it here because we have been using version .97 up until  
> now. We
> only recently upgraded to the head revision of log4cxx during the  
> process
> of moving our code to Visual Studio 2005(8).
>
> Sean Hermany
> Software Engineer
> FactSet Research Systems Inc.
> shermany@factset.com | 650.286.4942
> http://www.factset.com