[SSSD] [PATCHES] New DEBUG facility

Stephen Gallagher sgallagh at redhat.com
Mon Aug 8 13:28:44 UTC 2011


On Mon, 2011-08-08 at 07:51 -0400, Pavel Brezina wrote:
> https://fedorahosted.org/sssd/ticket/925
> 
> [PATCH 1/2] There are several new macros in util/util.h:
> 
> - SSSDBG_* to reflect a debug level (same names as in the ticket) -
> please,   don't use magic numbers anymore
> 
> - DEBUG_MSG(level, function, message) which will format the debug
> message   like "(time) [prg_name] [function] (level): message\n"
> 
> - DEBUG_IS_SET(level) that you should use to check if the level is
> allowed   to be logged   You can use it like: if
> (DEBUG_IS_SET(SSSDBG_TRACE_LIBS)) {...}
> 

For the most part, this patch is good (one minor nitpick: the comment
around DEBUG_MSG identifies the DEBUG macro, not the DEBUG_MSG macro)

However, it is very difficult to review. Please break this patch up into
several:

1) Creation of the new levels and the macros for converting the old
versions to them.
2) Modification of the DEBUG() macro and creation of the DEBUG_MSG()
macro
3) Conversion of the old DEBUG macros to the new levels
4) The unit tests.

This will make it much more digestible for the reviewer.





> [PATCH 2/2] Changes unresolved debug level value (SSSDBG_UNRESOLVED)
> from -1 to 0 so DEBUG macro could be reduced by one condition. Anyway,
> it has a minor effect, every time you want to load debug_level from
> command line parameters, you have to use following pattern:
> 
>     /* Set debug level to invalid value so we can deside if -d 0 was
> used. */     debug_level = SSSDBG_INVALID;
> 
>     pc = poptGetContext(argv[0], argc, argv, long_options, 0);    
> while((opt = poptGetNextOpt(pc)) != -1) { ... }
> 
>     debug_level = debug_level != SSSDBG_INVALID                   ?
> debug_convert_old_level(debug_level)                   :
> SSSDBG_UNRESOLVED; /* Debug level should be loaded from config file. */

I don't actually see any value to this patch. Also, wouldn't this
conflict with compatibility with old settings if 'debug_level = 0' had
been explicitly set?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20110808/7eeff7c3/attachment.sig>


More information about the sssd-devel mailing list