[SSSD] Imperfection of new debug levels macros

Jakub Hrozek jhrozek at redhat.com
Thu Nov 3 08:10:50 UTC 2011


On Wed, Nov 02, 2011 at 05:46:11PM -0400, Dmitri Pal wrote:
>    On 11/02/2011 01:04 PM, Stephen Gallagher wrote:
> 
>  On Wed, 2011-11-02 at 17:03 +0100, Jakub Hrozek wrote:
> 
>  On Wed, Nov 02, 2011 at 04:56:02PM +0100, Pavel Brezina wrote:
> 
>  Hi all,
>  while working on the sudo I've noticed some semantic issues with debug
>  level names.
> 
>  The debug levels are defined as:
> 
>  #define SSSDBG_FATAL_FAILURE  0x0010   /* level 0 */
>  #define SSSDBG_CRIT_FAILURE   0x0020   /* level 1 */
>  #define SSSDBG_OP_FAILURE     0x0040   /* level 2 */
>  #define SSSDBG_MINOR_FAILURE  0x0080   /* level 3 */
>  #define SSSDBG_CONF_SETTINGS  0x0100   /* level 4 */
>  #define SSSDBG_FUNC_DATA      0x0200   /* level 5 */
>  #define SSSDBG_TRACE_FUNC     0x0400   /* level 6 */
>  #define SSSDBG_TRACE_LIBS     0x1000   /* level 7 */
>  #define SSSDBG_TRACE_INTERNAL 0x2000   /* level 8 */
>  #define SSSDBG_TRACE_ALL      0x4000   /* level 9 */
> 
>  Unfortunately there are some old DEBUG call that does not correspond
>  with these names. Look at this snippet from responder initialization:
> 
>       if (ret != EOK) {
>           DEBUG(0, ("Failed to set up automatic
>                      reconnection\n"));
>           return ret;
>       }
> 
>       for (iter = sctx->rctx->be_conns; iter; iter = iter->next) {
>           sbus_reconnect_init(iter->conn, max_retries,
>                               sudo_dp_reconnect_init, iter);
>       }
> 
>       DEBUG(1, ("SUDO Initialization complete\n"));
> 
>  It is ok to replace the 0 with SSSDBG_FATAL_FAILURE (however I would
>  prefer SSSDBG_CRIT_FAILURE as it does not prevent reponder from starting).
>  But more important is - I can't use SSSDBG_CRIT_FAILURE, because it is
>  semantically nonsense. There should be SSSDBG_TRACE_FUNC. But if I use
>  it, it will differ from other responders and that would be just confusing.
> 
>  I think we can also set up an "alias":
> 
>  #define SSSDBG_IMPORTANT_INFO 0x0020
> 
> 
>  I think Pavel is correct that it should be SSSDBG_TRACE_FUNC here.
> 
>  When we made the conversion to the macros, we didn't change existing
>  values because it would have been a prohibitively large effort for
>  dubious gain.
> 
>  The decision we made was that if you find yourself working in a
>  particular area, you will convert any existing DEBUG messages in that
>  area as you go.
> 
>  So please feel free to change this here. We'll get the other responders
>  later so they agree.
> 
>  The reason we switched to these macros in the first place was because
>  everyone had a different idea of what each of the numeric levels should
>  mean. We're already wildly inconsistent throughout the code, so we might
>  as well start cleaning up pieces as we go.
> 
>    Do we have enough gidance to make them consistent somewhere laid out on
>    wiki so that everybody could follow same rules?
>    The worst would be if the re-factoring will change the values with no
>    benefit as one inconsistency would be replaced with another.
> 

Hopefully the macro names in util.h should be self-explanatory. A more
verbose guidance is in the sssd.conf man page. 



More information about the sssd-devel mailing list