Re: LDAP Debug messages & severity levels

[Hallvard B Furuseth <h.b.furuseth@usit.uio.no>]

Pierangelo Masarati writes:
>On Fri, 2006-01-20 at 22:15 +0100, Hallvard B Furuseth wrote:
>>   /* Helper macro for conditional logging - Info, Error. */
>>   #define LDAP_LOGC( level, logmask, args ) do { \
>> 	do { \
>> 		if ( ldap_debug & (logmask) ) \
>> 			lutil_debug( ldap_debug, (logmask), args ); \
>> 		if ( ldap_syslog & (level) ) \
>> 			syslog( LDAP_LEVEL_MASK((severity)), args ); \
>> 	} while ( 0 )
>>
>>   /* Used to send several arguments to one LDAP_LOG[UC] argument */
>>   #define LDAP_ARG ,
>
> I don't like this approach too much; in general, I think that trying to
> emulate variable arg number in macros leads to confusion.

It's a helper macro, only used by the logging macros and never
elsewhere.  I don't care if macro packages get a few extra hacks,
since they are ugly anyway with do{}while(0), () around params etc.

So I think a little trick like that is well worth getting rid of the
excess unused arguments.  But it's no big deal.

And in my opinion it's also much better than to get rid of the excess
args by duplicating the body of these macros.

>>   /* Conditional logging, always compiled in - because Statslog()
>>    * will be like Info() and will always be compiled in. */
>
> I think we can safely remove Statslog (that's what I did) as there is no
> difference between it and regular logging; it used to require conn & op,
> which makes little sense as it was being used as a Debug() with two more
> args...

Yes, I guess it should be Info5() in my model.

> As I said, I prefer to have one macro with an arg that identifies the
> severity rather than multiple macros, one for each severity, but it's a
> matter of taste.  We can always wrap one approach into the other,
> although there would be no clear advantage and things might even get
> more confusing.

Actually there is, I forgot to include part of the rationale for this
code from the previous discussion.

Today's slapd & tools has some design bugs with debugging vs logging:

configure --disable-debug should only disable debugging, not logging and
error messages.  Those are different beasts.  Also, even LDAP_DEBUG_ANY
does not make the output unconditional.  It is not output to stderr
unless the -d flag is given.  So we need at least three sets of macros:
The Debug macros and Info macros which obey the debug level/loglevel,
and the unconditional ones which do not.

Though an alternative could be to always set one bit in the ldap_debug
and ldap_syslog variables, so that the '... & LDAP_DEBUG_ANY' will
always be true.  Come to think of it, maybe my macros will need
something like that anyway since they call lutil_debug().  Ehh...  I
need to look at that.  Not sure what the effect would be of such a hack.


BTW, I suspect I'm in a bit of trouble with the severity level argument
as selector in the macro vs. syslog level and the slapd command line
arguments, but I'm a bit too tired at the moment to look at that...

-- 
Hallvard