On Wed, 2011-11-02 at 17:03 +0100, Jakub Hrozek wrote:
On Wed, Nov 02, 2011 at 04:56:02PM +0100, Pavel Březina 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.