On 11/8/05, John Torjo <john.lists@torjo.com> wrote:
- enabled_logger::enabled_logger (logger&): initialize m_stream in the member initialization or the ctor body but not both
Initialized it in member initialization for just in case 'm_stream = new logging_types::stream ;' could throw.
In which case your object construction will fail in the member initializers and not the body. Its the same effect - the caller will get a std::bad_alloc and the object will not be created. I don't see how initializing the member twice changes this.
- Remove old versions of macro definitions that have been rewritten
(BOOST_IS_LOG_ENABLED, BOOST_SCOPED_LOGL).
BOOST_IS_LOG_ENABLED - I wanted to still allow this for when you want to clearly state you're talking about a log. I think it's no harm.
BOOST_SCOPEDLOGL - when you want to use a scoped log, and instead of saying: if ( gui(my_level) ) gui.stream() << blabla;
I just meant that there are older definitions left in the code, commented out, above the current definitions. Is this what you want? from log.h (w/line numbers): 390://#define BOOST_SCOPEDLOGL(log_name,lvl) if (::boost::logging::simple_logger_keeper keep = ::boost::logging::simple_logger_keeper(log_name,::boost::logging::level::lvl) ) ; else (*keep.stream()) 391:#define BOOST_SCOPEDLOGL(log_name,lvl) if (::boost::logging::simple_logger_keeper keep = ::boost::logging::simple_logger_keeper(::boost::logging::logger_and_level(log_name,::boost::logging::level::lvl)) ) ; else (*keep.stream()) Again, I don't think it hurts, for consistency's sake, at least.
boost/log/log_impl.hpp: DEFAULT_INDEX should probably be lower-cased and possibly renamed. It should probably use BOOST_STATIC_CONSTANT.
boost/log/log_manager.hpp: DEFAULT_CACHE_LIMIT: See concerns above.
The reason they're uppercased is because they're constants.
But they're not macros. All-uppercase just screams macro to me.
I'd recommend getting rid of the prepend_time functor with its $xx format and use the strftime-based one instead. It might be also be nice to provide
The reason I prefer the prepend_time is because it's more straightforward IMO to say $dd (day), $MM (month), $yy (year 2 digits), $yyyy (year 4 digits) rather than %d (day), %m (month), %y (year 2 digits), %Y (year 4 digits). And the code is much easier to read, again, IMO.
I guess its just a point of preference, but I'm very familiar with strftime and its format strings, and it is part of the Standard C Library. Not sure which code you're referring to, but the implementation of prepend_time_stf is only about 15 lines.
Also, in general, do the loggers really need the m_destroyed member? Why
isn't the shared_ptr in m_impl enough to ensure the object lifetimes?
It's because internally, when writing BOOST_LOG(x) you're dealing with a function that returns a static reference.
The problem comes when you're using the static reference after it's been destroyed (thus the shared_ptr would be invalid)
This might be an argument of making the BOOST_LOG* macros work like this then: BOOST_LOG(logger, msg << to << stream) so that the implementation could hold a shared pointer to the logger for the lifetime of the call. Just a thought. Thanks! Welks :) -- Caleb Epstein caleb dot epstein at gmail dot com