Howard Chu writes: >Hallvard B Furuseth wrote: >> Not sure how the loglevel and severity level should interact. >> The most severe messages should be given independently of >> loglevel, so maybe their macros do not need a loglevel. >> >> With the others - log the message if both loglevel and >> severity level is satisfied? That's inconsistent with severe >> messages that are logged just due to severity level though. >> Maybe it'll be easier to decide after the debug messages >> have been classified by severity. > > I think you're going the right direction. Genuine Error and Warning > messages should not need any further qualification. It is only > Info/Debug messages that really need the fine-grain "loglevel" > differentiators, so they can be selectively turned on and off.
OK. Well, actually I'm not sure about warnings. I certainly appreciate fine-control warning options in some other programs. But that can wait in any case: I'd prefer a loglevel argument even in macros that don't use it at least until the code stabilizes, so a simple s/Warn/Info/ will be enough to downgrade a message from a severity level which doesn't use the loglevel to one which does. > There are some parts of the documentation that state that Statslog is > always available, even with --disable-debug. I think that doc is now > wrong, but it reflects an intention that we should probably restore. > That pretty much means Stats/Info, Warn, and Error are never disabled. OK. Should the default loglevel be 0 instead of Stats with --disable-debug - at least in OpenLDAP 2.3? Different defaults from different configurations can be confusing, but OTOH that will by now be the smallest change for current installations that use --disable-debug. >> include/ldap_log.h #defines twelve LDAP_LEVEL_* constants, >> but they only seem to be used in the unused debug2syslog(). > > Left over from NEW_LOGGING ? Ah, of course. From ldap_log.h revision 1.19 "first try at logging improvements". Well, they could just as well be left alone for now. Might need them soon. A few other points: Should we take the opportunity to get rid of "too many arguments for format" warnings? Introduce <Debug,Info,Warn,Err,Statslog><1,2,3,4,5>() macros that take 1-5 arguments. I believe they'd all be one-liners calling the same general logging macro. Well, maybe Statslog*() would keep a separate macro. The old Debug() and Statslog() APIs would also stay, at least for now. We could also switch to using Debug/Err/...( SOMELEVEL, ... ) instead of Debug( LDAP_DEBUG_SOMELEVEL, ...) and have the macros prepend LDAP_DEBUG_ to SOMELEVEL, as NEW_LOGGING did. As a practical matter, this change could be introduced behind #ifdef SEVERITY_LOGGING /* defined when LDAP_DEVEL */ # define Debug(), Info(), Err() etc to do just what Debug() does now #else # define them to obey a configurable severity level, and #define all # except Debug() even when --disable-debug. #endif That way there should be is no need for a PITA like #ifdef NEW_LOGGING everywhere in the code, nor a separate CVS branch (which I imagine would have a lot of conflicts by the time we were ready to merge it into HEAD). -- Hallvard
