[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