[SSSD] [PATCHES] New DEBUG facility

Jan Zelený jzeleny at redhat.com
Tue Aug 16 13:20:36 UTC 2011


> Rebased to current master
> (2011-08-15 17:46 Jakub Hrozek 5bf2314b9f64099cd4e88b8f3498d986d97e1ac6)

I have some questions and comments. In general, the patches look fine, but 
there are couple things (mostly cosmetic) which I'd like to get right.

How about mask (#define) adding only failures / only traces / ... ? Dou you 
think it would make sense to add them or not?

Commit comments - the same comment in more patches is a little confusing. 
Please leave only the part relevant for the patch.


Patch #0001:
- why are there two functions handling almost the same thing differently?
  (debug_convert_old_level and debug_get_level)

Patch #0002:
- lines 45, 76, 96: the first part of condition is redundant
  (removed in patch #0006 - please merge that part with this patch)
- please use DEBUG_IS_SET in DEBUG_MSG and DEBUG

Patch #0003:
- line 166: I'd like to avoid changing these DEBUG statements like this
  -- either change them all or none, not just the one
  -- the same for lines 212, 228, 313

Patch #0004:
- I have to read through it yet, perhaps a native english speaker should take 
a look as well

Patch #0005:
- I'll trust you on this one if all tests pass ;-)

Patch #0006:
- <nitpick> please just for safety use parentheses in ternary operator (line 
92 and couple more similar)</nitpick>
- why don't you remove SSS_UNRESOLVED_DEBUG_LEVEL entirely?

I'm running some more tests now, but I think I won't find any further issues.

Jan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20110816/1d5257f0/attachment.sig>


More information about the sssd-devel mailing list