[SSSD] Imperfection of new debug levels macros

Dmitri Pal dpal at redhat.com
Wed Nov 2 21:46:11 UTC 2011


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 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.

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.

>
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://fedorahosted.org/mailman/listinfo/sssd-devel


-- 
Thank you,
Dmitri Pal

Sr. Engineering Manager IPA project,
Red Hat Inc.


-------------------------------
Looking to carve out IT costs?
www.redhat.com/carveoutcosts/



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20111102/a7530f72/attachment.html>


More information about the sssd-devel mailing list